Skip to content

Commit e39cf04

Browse files
authored
feat(gradle-server): migrate task transport from gRPC to JSON-RPC over TCP loopback (PR 1/3) (#1863)
* feat(gradle-server): migrate task transport from gRPC to JSON-RPC over TCP loopback Replace the existing gRPC + Netty HTTP/2 transport for the task server with LSP4J JSON-RPC over a plain TCP loopback socket. This eliminates the mid-frame HTTP/2 race (vscode-gradle issue #1815) at its source by removing the Netty stack from the task-server hot path entirely. What changes on the gradle-server side - New package ransport.jsonrpc exposing six methods on gradle/ segment: getBuild, runBuild, getProjectDependencies, cancelBuild, cancelBuilds, executeCommand. Streaming RPCs (getBuild/runBuild) deliver intermediate progress through gradle/getBuild/reply and gradle/runBuild/reply notifications and complete the JSON-RPC request with the terminal reply. - Protobuf wire schema (proto/gradle.proto) is reused for payload bytes; request/response/notification params carry the base64-encoded proto bytes plus a stream id. This keeps the message layer stable across the migration and avoids a JSON re-encoding of large progress payloads. - GradleServer becomes a thin bootstrap: it connects out to the Node listener on 127.0.0.1:<port>, builds an LSP4J launcher backed by a cached worker pool, and blocks until the socket closes. - Handlers no longer hold gRPC StreamObservers. Streaming handlers take (Request, CompletableFuture<GradleResponse>, GradleClient, long streamId); unary handlers take (Request, CompletableFuture<GradleResponse>). Removed - io.grpc:grpc-protobuf, grpc-stub, grpc-netty (plus grpc-testing) dependencies, the protoc-gen-grpc-java plugin, and the �uild/generated/source/proto/main/grpc source set. - TaskService (gRPC service shim) and ErrorMessageBuilder (only used by the gRPC error path). - The Netty HTTP/2 mid-frame log filter and its tests (GradleServerFilterTest). - Forced etty-bom pin (no longer relevant since Netty is gone from the task-server runtime). Tests - GradleServerTest now exercises handlers directly via CompletableFuture<GradleResponse> and a Mockito-mocked GradleClient, so the test surface verifies Gradle Tooling API interactions without spinning up a gRPC InProcess channel. - One extra --add-opens (java.util.concurrent) is added to the test JVM args so PowerMock can introspect CompletableFuture on JDK 17+. This commit is the gradle-server half of the migration. The TypeScript extension still speaks gRPC and is updated in PR 2; the proto files and the root-build grpcVersion constant are cleaned up in PR 3. * test(gradle-server): cover JSON-RPC transport with end-to-end roundtrip tests Add JsonRpcTransportTest exercising the new transport package end-to-end over a pair of piped streams. Two Launcher instances are wired together (server side: GradleService stub + GradleClient remote proxy; client side: recording GradleClient + GradleService remote proxy) so the full request/response/notification path is driven without a real socket and without a dependency on the Gradle Tooling API. Coverage: - Base64 protobuf encode/decode roundtrip via JsonRpcCodec. - All six RPC methods (getBuild, unBuild, getProjectDependencies, cancelBuild, cancelBuilds, �xecuteCommand) — payload identity, stream-id propagation, and null eply handling. - Streaming notifications: getBuild/reply carries the right stream id and payload to the client side; unBuild/reply is delivered on the separate unBuild notification channel without leaking into the getBuild channel. - Error mapping across the wire for ResponseErrorException produced by JsonRpcCodec.error(...) — covers NOT_FOUND, CANCELLED and INTERNAL (the last one via a Throwable carrier). - Pinned constants for the four error codes that form part of the wire contract with the TS client, to catch accidental renumbering. The handler-level behaviour (Gradle Tooling API interaction, debug init script, JVM args, etc.) stays covered by GradleServerTest, which talks to handlers directly via CompletableFuture<GradleResponse> rather than through the JSON-RPC dispatch layer — so the two test classes between them cover the wire layer and the business logic without overlapping. * fix: update the review comments * fix(gradle-server): address Copilot review comments on PR #1863 - GradleServiceImpl: validate JSON-RPC params/request nullability and map Base64 decode errors to ERROR_UNKNOWN instead of ERROR_INTERNAL so client-side input bugs surface with the correct wire code. - ExecuteCommandHandler: split error replies so unknown commands return ERROR_NOT_FOUND and argument-count violations return ERROR_UNKNOWN; reserve ERROR_INTERNAL for genuine server failures. - GradleServer: extract a workerThreadFactory() that hands out unique `gradle-jsonrpc-worker-N` names via AtomicInteger so concurrent handlers are distinguishable in thread dumps and logs. Adds regression coverage: - JsonRpcTransportTest: 4 new tests for null params, null request, invalid Base64, and null cancelBuilds params. - ExecuteCommandHandlerTest (new): 4 tests covering the new error code mapping plus the happy path. - GradleServerThreadFactoryTest (new): 3 tests covering unique naming, daemon flag, and per-instance counter isolation. * perf: improve sync lock and order code * perf: error code * chore: ignore local-only *.local.md working notes
1 parent e916b25 commit e39cf04

24 files changed

Lines changed: 1456 additions & 527 deletions

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,6 @@ gradle-server/src/main/resources/*.jar
3333

3434
# Ignore user-specified .tool-versions file (asdf-vm configuration)
3535
.tool-versions
36+
37+
# Local-only working notes (never commit)
38+
*.local.md

gradle-server/build.gradle

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,22 +18,18 @@ repositories {
1818
}
1919

2020
dependencies {
21-
implementation enforcedPlatform('io.netty:netty-bom:4.1.130.Final')
2221
implementation project(":gradle-plugin-api")
2322
implementation files('build/libs/server.jar')
2423
implementation fileTree(dir: 'build/libs/runtime', include: ['*.jar'], exclude: ['gradle-tooling-api-*.jar',
2524
'slf4j-api-*.jar'])
2625
implementation("org.gradle:gradle-tooling-api:${toolingAPIVersion}")
2726
implementation 'javax.annotation:javax.annotation-api:1.3.2'
28-
implementation "io.grpc:grpc-protobuf:${grpcVersion}"
29-
implementation "io.grpc:grpc-stub:${grpcVersion}"
30-
implementation "io.grpc:grpc-netty:${grpcVersion}"
27+
implementation "com.google.protobuf:protobuf-java:${protobufVersion}"
3128
implementation 'io.github.g00fy2:versioncompare:1.4.1'
3229
implementation project(':gradle-language-server')
3330
implementation "org.eclipse.lsp4j:org.eclipse.lsp4j:0.24.0"
3431
implementation "org.eclipse.lsp4j:org.eclipse.lsp4j.jsonrpc:0.24.0"
3532
runtimeOnly 'org.slf4j:slf4j-simple:2.0.0-alpha6'
36-
testImplementation "io.grpc:grpc-testing:${grpcVersion}"
3733
testImplementation 'junit:junit:4.13.1'
3834
testImplementation 'org.powermock:powermock-module-junit4:2.0.9'
3935
testImplementation 'org.powermock:powermock-api-mockito2:2.0.7'
@@ -51,7 +47,6 @@ sourceSets {
5147
}
5248
java {
5349
srcDirs mainJavaDir
54-
srcDirs 'build/generated/source/proto/main/grpc'
5550
srcDirs 'build/generated/source/proto/main/java'
5651
}
5752
resources {
@@ -61,25 +56,12 @@ sourceSets {
6156
}
6257

6358
protobuf {
64-
plugins {
65-
grpc {
66-
// for apple m1, please add protoc_platform=osx-x86_64 in $HOME/.gradle/gradle.properties
67-
if (project.hasProperty('protoc_platform')) {
68-
artifact = "io.grpc:protoc-gen-grpc-java:${grpcVersion}:${protoc_platform}"
69-
} else {
70-
artifact = "io.grpc:protoc-gen-grpc-java:${grpcVersion}"
71-
}
72-
}
73-
}
7459
generateProtoTasks {
7560
generateTestProto.enabled = false
7661
extractProto.enabled = false;
7762
extractIncludeProto.enabled = false;
7863
extractIncludeTestProto.enabled = false;
7964
all().each { task ->
80-
task.plugins {
81-
grpc {}
82-
}
8365
task.builtins {
8466
remove distribution
8567
}
@@ -157,6 +139,7 @@ project.tasks.named("processResources") {
157139
test {
158140
jvmArgs '--add-opens=java.base/java.lang=ALL-UNNAMED',
159141
'--add-opens=java.base/java.util=ALL-UNNAMED',
142+
'--add-opens=java.base/java.util.concurrent=ALL-UNNAMED',
160143
'--add-opens=java.base/java.lang.reflect=ALL-UNNAMED',
161144
'--add-opens=java.base/java.nio.file=ALL-UNNAMED',
162145
'--add-opens=java.base/java.io=ALL-UNNAMED',

gradle-server/src/main/java/com/github/badsyntax/gradle/ErrorMessageBuilder.java

Lines changed: 0 additions & 17 deletions
This file was deleted.
Lines changed: 40 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -1,71 +1,24 @@
11
package com.github.badsyntax.gradle;
22

3+
import com.github.badsyntax.gradle.transport.jsonrpc.TaskSocketServer;
34
import com.github.badsyntax.gradle.utils.Utils;
45
import com.google.common.base.Strings;
56
import com.microsoft.gradle.GradleLanguageServer;
6-
import io.grpc.Server;
7-
import io.grpc.ServerBuilder;
8-
import io.grpc.StatusRuntimeException;
9-
import io.grpc.netty.NettyServerBuilder;
10-
import java.io.IOException;
11-
import java.net.InetSocketAddress;
127
import java.util.Map;
13-
import java.util.concurrent.TimeUnit;
14-
import java.util.logging.Filter;
15-
import java.util.logging.Handler;
16-
import java.util.logging.Level;
8+
import java.util.concurrent.ExecutorService;
9+
import java.util.concurrent.Executors;
10+
import java.util.concurrent.Future;
11+
import java.util.concurrent.atomic.AtomicInteger;
1712
import org.slf4j.Logger;
1813
import org.slf4j.LoggerFactory;
1914

2015
public class GradleServer {
2116
private static final Logger logger = LoggerFactory.getLogger(GradleServer.class.getName());
2217

23-
private final int port;
24-
private final Server taskServer;
25-
26-
public GradleServer(int port) {
27-
this(NettyServerBuilder.forAddress(new InetSocketAddress("127.0.0.1", port)), port);
28-
}
29-
30-
public GradleServer(ServerBuilder<?> serverBuilder, int port) {
31-
this.port = port;
32-
taskServer = serverBuilder.addService(new TaskService()).build();
33-
}
34-
35-
@SuppressWarnings("java:S106")
36-
public void start() throws IOException {
37-
taskServer.start();
38-
logger.info("Gradle Server started, listening on {}", port);
39-
Runtime.getRuntime().addShutdownHook(new Thread() {
40-
@Override
41-
public void run() {
42-
logger.info("Shutting down gRPC server since JVM is shutting down");
43-
try {
44-
GradleServer.this.stop();
45-
} catch (InterruptedException e) {
46-
e.printStackTrace(System.err);
47-
Thread.currentThread().interrupt();
48-
}
49-
logger.info("Server shut down");
50-
}
51-
});
52-
}
53-
54-
public void stop() throws InterruptedException {
55-
if (taskServer != null) {
56-
taskServer.shutdown().awaitTermination(30, TimeUnit.SECONDS);
57-
}
58-
}
59-
60-
private void blockUntilShutdown() throws InterruptedException {
61-
if (taskServer != null) {
62-
taskServer.awaitTermination();
63-
}
18+
private GradleServer() {
6419
}
6520

6621
public static void main(String[] args) throws Exception {
67-
installNettyMidFrameWarningFilter();
68-
6922
Map<String, String> params = Utils.parseArgs(args);
7023

7124
int taskServerPort = Integer.parseInt(Utils.validateRequiredParam(params, "port"));
@@ -84,63 +37,30 @@ public static void main(String[] args) throws Exception {
8437
}
8538
}
8639

87-
/**
88-
* The client retries once on the known Node.js http2 race that produces a
89-
* truncated DATA + END_STREAM frame when reusing an HTTP/2 session (tracked
90-
* upstream as grpc/grpc-node#2872; maintainers attribute the root cause to Node
91-
* and recommend application-level retry). When this happens, grpc-netty logs an
92-
* INTERNAL "Encountered end-of-stream mid-frame" WARNING with a long stack
93-
* trace to stderr, which surfaces in the extension output channel as a scary
94-
* error even though the retried call succeeded. Filter that single record out
95-
* of every JUL handler attached to the root logger; the filter checks the log
96-
* level, the logger name, the throwable type and the message text, so all other
97-
* Netty warnings (TLS, protocol violations, etc.) and any record at a different
98-
* level (e.g. SEVERE) pass through unchanged. If a handler already had a filter
99-
* configured, the existing filter is preserved and chained so we never silently
100-
* bypass other logging policies.
101-
*/
102-
private static void installNettyMidFrameWarningFilter() {
103-
Filter suppression = buildNettyMidFrameWarningFilter();
104-
java.util.logging.Logger root = java.util.logging.Logger.getLogger("");
105-
for (Handler h : root.getHandlers()) {
106-
Filter previous = h.getFilter();
107-
h.setFilter(composeFilters(previous, suppression));
108-
}
109-
}
110-
111-
// Package-private for testing.
112-
static Filter buildNettyMidFrameWarningFilter() {
113-
return record -> {
114-
if (!Level.WARNING.equals(record.getLevel())) {
115-
return true;
116-
}
117-
Throwable t = record.getThrown();
118-
String name = record.getLoggerName();
119-
return !(name != null && name.startsWith("io.grpc.netty.NettyServerStream")
120-
&& t instanceof StatusRuntimeException && t.getMessage() != null
121-
&& t.getMessage().contains("Encountered end-of-stream mid-frame"));
122-
};
123-
}
124-
125-
// Package-private for testing.
126-
static Filter composeFilters(Filter previous, Filter next) {
127-
if (previous == null) {
128-
return next;
129-
}
130-
return record -> previous.isLoggable(record) && next.isLoggable(record);
131-
}
132-
13340
private static void startTaskServerThread(int port) {
134-
GradleServer server = new GradleServer(port);
41+
ExecutorService workerExecutor = Executors.newCachedThreadPool(workerThreadFactory());
13542
Thread serverThread = new Thread(() -> {
43+
int exitCode = 0;
13644
try {
137-
server.start();
138-
server.blockUntilShutdown();
139-
} catch (IOException | InterruptedException e) {
140-
throw new RuntimeException(e);
45+
Future<Void> listening = TaskSocketServer.connectAndStart(port, workerExecutor);
46+
logger.info("Gradle Server JSON-RPC transport connected on loopback port {}", port);
47+
listening.get();
48+
logger.info("Gradle Server JSON-RPC transport closed");
49+
} catch (Exception e) {
50+
exitCode = 1;
51+
logger.error("Gradle Server JSON-RPC transport failed", e);
52+
} finally {
53+
workerExecutor.shutdownNow();
14154
}
142-
});
55+
logger.info("Exiting Gradle Server JVM because JSON-RPC task transport ended");
56+
System.exit(exitCode);
57+
}, "gradle-jsonrpc-server");
14358
serverThread.start();
59+
60+
Runtime.getRuntime().addShutdownHook(new Thread(() -> {
61+
logger.info("Shutting down Gradle Server JSON-RPC transport since JVM is shutting down");
62+
workerExecutor.shutdownNow();
63+
}, "gradle-jsonrpc-shutdown"));
14464
}
14565

14666
private static void startBuildServerThread(String pipeName, String directory) {
@@ -155,4 +75,19 @@ private static void startLanguageServerThread(String languageServerPipePath) {
15575
});
15676
languageServerThread.start();
15777
}
78+
79+
/**
80+
* Worker thread factory: gives every JSON-RPC worker thread a unique name
81+
* (suffixed with a monotonic counter) so thread dumps and logs can distinguish
82+
* concurrent handlers. Daemon threads so they don't keep the JVM alive past
83+
* {@link #main(String[])}. Package-private for testing.
84+
*/
85+
static java.util.concurrent.ThreadFactory workerThreadFactory() {
86+
AtomicInteger counter = new AtomicInteger();
87+
return runnable -> {
88+
Thread t = new Thread(runnable, "gradle-jsonrpc-worker-" + counter.incrementAndGet());
89+
t.setDaemon(true);
90+
return t;
91+
};
92+
}
15893
}

gradle-server/src/main/java/com/github/badsyntax/gradle/TaskService.java

Lines changed: 0 additions & 49 deletions
This file was deleted.

gradle-server/src/main/java/com/github/badsyntax/gradle/handlers/CancelBuildHandler.java

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,21 @@
44
import com.github.badsyntax.gradle.CancelBuildRequest;
55
import com.github.badsyntax.gradle.GradleBuildCancellation;
66
import com.github.badsyntax.gradle.exceptions.GradleCancellationException;
7-
import io.grpc.stub.StreamObserver;
7+
import com.github.badsyntax.gradle.transport.jsonrpc.GradleResponse;
8+
import com.github.badsyntax.gradle.transport.jsonrpc.JsonRpcCodec;
9+
import java.util.concurrent.CompletableFuture;
810
import org.slf4j.Logger;
911
import org.slf4j.LoggerFactory;
1012

1113
public class CancelBuildHandler {
1214
private static final Logger logger = LoggerFactory.getLogger(CancelBuildHandler.class.getName());
1315

1416
private CancelBuildRequest req;
15-
private StreamObserver<CancelBuildReply> responseObserver;
17+
private CompletableFuture<GradleResponse> response;
1618

17-
public CancelBuildHandler(CancelBuildRequest req, StreamObserver<CancelBuildReply> responseObserver) {
19+
public CancelBuildHandler(CancelBuildRequest req, CompletableFuture<GradleResponse> response) {
1820
this.req = req;
19-
this.responseObserver = responseObserver;
21+
this.response = response;
2022
}
2123

2224
public void run() {
@@ -26,18 +28,18 @@ public void run() {
2628
} catch (GradleCancellationException e) {
2729
logger.error(e.getMessage());
2830
replyWithCancelError(e);
29-
} finally {
30-
responseObserver.onCompleted();
3131
}
3232
}
3333

3434
private void replyWithCancelledSuccess() {
35-
responseObserver.onNext(
36-
CancelBuildReply.newBuilder().setMessage("Cancel build requested").setBuildRunning(true).build());
35+
CancelBuildReply reply = CancelBuildReply.newBuilder().setMessage("Cancel build requested")
36+
.setBuildRunning(true).build();
37+
response.complete(new GradleResponse(JsonRpcCodec.encode(reply)));
3738
}
3839

3940
private void replyWithCancelError(Exception e) {
40-
responseObserver
41-
.onNext(CancelBuildReply.newBuilder().setMessage(e.getMessage()).setBuildRunning(false).build());
41+
CancelBuildReply reply = CancelBuildReply.newBuilder().setMessage(e.getMessage()).setBuildRunning(false)
42+
.build();
43+
response.complete(new GradleResponse(JsonRpcCodec.encode(reply)));
4244
}
4345
}

0 commit comments

Comments
 (0)