Remove JSR-305 ThreadSafe annotation and replace with JavaDoc by Kainsin · Pull Request #12762 · grpc/grpc-java

Kainsin · 2026-04-17T13:02:40Z

This is another attempt to remove JSR-305 annotations but instead of replacing with ErrorProne's ThreadSafe, sticks to adding a JavaDoc comment. This should basically keep things inline with what JSR-305 ThreadSafe affords.

Adding ErrorProne's ThreadSafe can be considered in the future, as it expects more things than JSR-305.

Removing the JSR-305 dependency here allows Java applications that have moved away from javax to compile and avoids a bug in Immutables and Lombok (and possibly other annotation processors) from failing when JSR-305 is not present.

Kainsin · 2026-04-17T15:40:20Z

I am not sure why that test is failing - it's passing locally. Am I missing a configuration setting that is causing an extra log to be emitted here?

Kainsin · 2026-04-20T15:48:00Z

I was able to make a change to fix the test - it's in the second commit.

@kannanjgithub you reviewed my last PR for this, would you be able to review this one as well? Thanks!

kannanjgithub · 2026-04-27T05:45:07Z

 */
 @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1704")
-@ThreadSafe
 public final class CompressorRegistry {


The suggestion from Eric was only to replace the Threadsafe annotation for the public non-final classes. This and many other classes changed in this PR are final classes.

Understood, I'll fix that.

@kannanjgithub Sorry for taking so long to get back to this. I updated the commit and only public non-final classes should be included in the change.

Thanks!

kannanjgithub · 2026-05-12T10:02:36Z

There is an unrelated commit 9425fd3 with this PR.

Kainsin · 2026-05-12T14:47:54Z

There is an unrelated commit 9425fd3 with this PR.

@kannanjgithub There was a test passing locally but failing in CI. I added this to fix the race condition.

Copilot

Pull request overview

This PR removes usages of the JSR-305 javax.annotation.concurrent.ThreadSafe annotation across several gRPC public APIs and internal interfaces, replacing it with JavaDoc statements, with the goal of reducing reliance on JSR-305-provided annotations and improving compatibility for builds that don’t include them.

Changes:

Removed javax.annotation.concurrent.ThreadSafe imports/usages from multiple API, core, stub, and binder types and replaced them with JavaDoc text asserting thread-safety.
Updated OkHttpClientTransportTest teardown to wait briefly for executor termination after shutdown.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 28 comments.

Show a summary per file

File	Description
stub/src/main/java/io/grpc/stub/AbstractStub.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
stub/src/main/java/io/grpc/stub/AbstractFutureStub.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
stub/src/main/java/io/grpc/stub/AbstractBlockingStub.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
stub/src/main/java/io/grpc/stub/AbstractAsyncStub.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java	Improved executor shutdown behavior in `tearDown()`.
core/src/main/java/io/grpc/internal/ObjectPool.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
core/src/main/java/io/grpc/internal/ManagedClientTransport.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
core/src/main/java/io/grpc/internal/KeepAliveManager.java	Removed `@ThreadSafe` on nested interface, added JavaDoc thread-safety note.
core/src/main/java/io/grpc/internal/InternalServer.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
core/src/main/java/io/grpc/internal/ConnectionClientTransport.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
core/src/main/java/io/grpc/internal/ClientTransport.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
binder/src/main/java/io/grpc/binder/internal/BinderTransport.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
api/src/main/java/io/grpc/StreamTracer.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
api/src/main/java/io/grpc/ServerStreamTracer.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
api/src/main/java/io/grpc/ServerInterceptor.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
api/src/main/java/io/grpc/ServerCallHandler.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
api/src/main/java/io/grpc/Server.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
api/src/main/java/io/grpc/NameResolver.java	Removed `@ThreadSafe` on `Listener`, added JavaDoc thread-safety note.
api/src/main/java/io/grpc/ManagedChannel.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
api/src/main/java/io/grpc/LoadBalancer.java	Removed `@ThreadSafe` on nested types, added JavaDoc thread-safety notes.
api/src/main/java/io/grpc/HandlerRegistry.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
api/src/main/java/io/grpc/ClientStreamTracer.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
api/src/main/java/io/grpc/ClientInterceptor.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
api/src/main/java/io/grpc/ChannelLogger.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.
api/src/main/java/io/grpc/Channel.java	Removed `@ThreadSafe`, added JavaDoc thread-safety note.

Comments suppressed due to low confidence (7)

api/src/main/java/io/grpc/Channel.java:34

The new JavaDoc includes a whitespace-only line with trailing spaces and a tooling-specific TODO about using Error Prone’s @ThreadSafe later. Please remove trailing whitespace and keep the JavaDoc focused on the actual thread-safety contract.

 * 
 * <p>This is thread-safe and should be considered
 * for the errorprone ThreadSafe annotation in the future.
 */

api/src/main/java/io/grpc/ChannelLogger.java:27

The added JavaDoc includes a trailing-whitespace blank line and a tooling-specific TODO about adopting Error Prone’s @threadsafe. Please remove trailing whitespace and keep the public JavaDoc focused on the thread-safety contract rather than future tooling changes.

 * A Channel-specific logger provided by GRPC library to {@link LoadBalancer} implementations.
 * Information logged here goes to <strong>Channelz</strong>, and to the Java logger of this class
 * as well.
 * 
 * <p>This is thread-safe and should be considered
 * for the errorprone ThreadSafe annotation in the future.
 */

api/src/main/java/io/grpc/ClientInterceptor.java:22

This file now contains two blank lines between the package statement and the type JavaDoc after removing the last import. Please reduce to a single blank line to match the project’s usual formatting for no-import files (e.g., api/src/main/java/io/grpc/CallbackMetricInstrument.java:17-21).

This issue also appears on line 39 of the same file.

package io.grpc;


/**
 * Interface for intercepting outgoing calls before they are dispatched by a {@link Channel}.
 *

api/src/main/java/io/grpc/ClientInterceptor.java:42

The newly added JavaDoc includes a trailing-whitespace blank line and a tooling/TODO note about potentially switching to Error Prone’s @ThreadSafe in the future. Please remove trailing whitespace and keep the JavaDoc to the actual thread-safety contract.

 * 
 * <p>This is thread-safe and should be considered
 * for the errorprone ThreadSafe annotation in the future.
 */

api/src/main/java/io/grpc/ServerInterceptor.java:22

There are now two blank lines between the package declaration and the type JavaDoc (after removing the only import). Please reduce this to a single blank line (consistent with api/src/main/java/io/grpc/CallbackMetricInstrument.java:17-21).

package io.grpc;


/**
 * Interface for intercepting incoming calls before they are dispatched by
 * {@link ServerCallHandler}.

api/src/main/java/io/grpc/LoadBalancer.java:1036

The added JavaDoc includes a tooling/TODO note about adopting Error Prone’s @ThreadSafe later. Please keep this public API JavaDoc focused on the thread-safety contract, and remove the future tooling discussion (also ensure blank lines don’t contain trailing whitespace).

   * Provides essentials for LoadBalancer implementations.
   *
   * <p>This is thread-safe and should be considered
   * for the errorprone ThreadSafe annotation in the future.
   *

api/src/main/java/io/grpc/LoadBalancer.java:1559

This new JavaDoc block includes a tooling-focused TODO about potentially switching to Error Prone’s @threadsafe. Since this is part of the public API docs, please replace it with a direct thread-safety guarantee/requirement and remove the tooling discussion (and avoid trailing whitespace on blank lines).

  /**
   * Factory to create {@link LoadBalancer} instance.
   *
   * <p>This is thread-safe and should be considered
   * for the errorprone ThreadSafe annotation in the future.
   *

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+      executor.shutdownNow();
+      executor.awaitTermination(10, TimeUnit.SECONDS);
+    } catch (InterruptedException e) {
+        // Ignore in a test and continue on as normal.


+ * <p>This is thread-safe and should be considered
+ * for the errorprone ThreadSafe annotation in the future.


+ * <p>This is thread-safe and should be considered
+ * for the errorprone ThreadSafe annotation in the future.


+ * <p>This is thread-safe and should be considered
+ * for the errorprone ThreadSafe annotation in the future.


+ * <p>This is thread-safe and should be considered
+ * for the errorprone ThreadSafe annotation in the future.


+ * 
+ * <p>This is thread-safe and should be considered
+ * for the errorprone ThreadSafe annotation in the future.


+ * 
+ * <p>This is thread-safe and should be considered
+ * for the errorprone ThreadSafe annotation in the future.


+   * 
+   * <p>This is thread-safe and should be considered
+   * for the errorprone ThreadSafe annotation in the future.
+   * 


Kainsin · 2026-05-13T13:12:02Z

 import java.util.concurrent.Executor;
 import java.util.concurrent.ScheduledExecutorService;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
-import javax.annotation.concurrent.ThreadSafe;



The commit message is only for removing the ThreadSafe dependency. It will not be removed everywhere, though. Only in public, non-final classes and interfaces.

+   * 
+   * <p>This is thread-safe and should be considered
+   * for the errorprone ThreadSafe annotation in the future.


This fixes a potential race condition where a test runs a thread in the executor that does some logging. Without waiting for threads to finish, another test can be run while that background thread is still executing. This can cause flaky issues with tests that only think that their test code is executing.

Kainsin force-pushed the master branch from 24991fb to 650603d Compare April 17, 2026 13:35

Kainsin marked this pull request as ready for review April 17, 2026 15:39

Kainsin force-pushed the master branch 2 times, most recently from 951018b to 5fbc141 Compare April 20, 2026 15:20

kannanjgithub reviewed Apr 27, 2026

View reviewed changes

Kainsin force-pushed the master branch 2 times, most recently from 417527f to 9425fd3 Compare May 11, 2026 15:06

kannanjgithub requested a review from Copilot May 13, 2026 06:33

Copilot started reviewing on behalf of kannanjgithub May 13, 2026 06:34 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

Kainsin added 2 commits May 13, 2026 09:09

Remove JSR-305 ThreadSafe annotation and replace with JavaDoc

481b517

Kainsin force-pushed the master branch from 9425fd3 to 3501d20 Compare May 13, 2026 13:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove JSR-305 ThreadSafe annotation and replace with JavaDoc#12762

Remove JSR-305 ThreadSafe annotation and replace with JavaDoc#12762
Kainsin wants to merge 2 commits into
grpc:masterfrom
Kainsin:master

Kainsin commented Apr 17, 2026

Uh oh!

Kainsin commented Apr 17, 2026

Uh oh!

Kainsin commented Apr 20, 2026

Uh oh!

kannanjgithub Apr 27, 2026

Uh oh!

Kainsin Apr 27, 2026

Uh oh!

Kainsin May 11, 2026

Uh oh!

kannanjgithub commented May 12, 2026

Uh oh!

Kainsin commented May 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Kainsin May 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		* <p>This is thread-safe and should be considered
		* for the errorprone ThreadSafe annotation in the future.

Conversation

Kainsin commented Apr 17, 2026

Uh oh!

Kainsin commented Apr 17, 2026

Uh oh!

Kainsin commented Apr 20, 2026

Uh oh!

kannanjgithub Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Kainsin Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Kainsin May 11, 2026

Choose a reason for hiding this comment

Uh oh!

kannanjgithub commented May 12, 2026

Uh oh!

Kainsin commented May 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Kainsin May 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants