Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
jre: [8, 11, 17]
jre: [8, 11, 17, 21]
fail-fast: false # Should swap to true if we grow a large matrix

steps:
Expand Down
2 changes: 2 additions & 0 deletions api/src/main/java/io/grpc/StatusException.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
*/
public class StatusException extends Exception {
private static final long serialVersionUID = -660954903976144640L;
@SuppressWarnings("serial")
private final Status status;
@SuppressWarnings("serial")
private final Metadata trailers;

/**
Expand Down
2 changes: 2 additions & 0 deletions api/src/main/java/io/grpc/StatusRuntimeException.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
public class StatusRuntimeException extends RuntimeException {

private static final long serialVersionUID = 1950934672280720624L;
@SuppressWarnings("serial")
private final Status status;
@SuppressWarnings("serial")
private final Metadata trailers;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ void writeFrame(
*/
private volatile boolean cancelled;

@SuppressWarnings("this-escape")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a single this-escape was fixed. Are you thinking we should just disable the warning for the entire project? Otherwise, we might want to allow it on particular lines, which we feel are safe. You can do that via something like:

@SuppressWarnings("this-escape")
MessageFramer framer = new MessageFramer(this, bufferAllocator, statsTraceCtx);
this.framer = framer;

(That won't work in all cases, but would work in most it looks like. It wouldn't work on setCumulator())

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compiler is giving warnings even with this format. It is asking to suppress "this-escape" at constructor level.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disable the warning for the entire project?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm now leaning toward keeping it enabled, as it looks like it generally tells us we are doing something wrong. I do agree the check is a bit overzealous, but it requires a public, non-final class. We shouldn't be doing that, as the API is quite hard to get right. But all of these are @Internal, and they are actually outliers. I think it may make sense to silence them here, so if someone adds a new instance of the warning it'll be more obvious. It's actually a pretty good tell-tale that the class should be package-private and/or final.

The one semi-exception to "it is @Internal" is MultiChildLoadBalancer. It is internal today, but is intended to be public in the future. And it should be fixed before it is used more widely.

Copy link
Member Author

@shivaspeaks shivaspeaks Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one semi-exception to "it is @Internal" is MultiChildLoadBalancer. It is internal today, but is intended to be public in the future. And it should be fixed before it is used more widely.

So we should open a tracking issue for this? Or is there any already pre-existing issue for making MultiChildLoadBalancer public then we may add a comment on that mentioning this? I couldn't find one.

protected AbstractClientStream(
WritableBufferAllocator bufferAllocator,
StatsTraceContext statsTraceCtx,
Expand All @@ -113,7 +114,7 @@ protected AbstractClientStream(
this.shouldBeCountedForInUse = GrpcUtil.shouldBeCountedForInUse(callOptions);
this.useGet = useGet;
if (!useGet) {
framer = new MessageFramer(this, bufferAllocator, statsTraceCtx);
this.framer = new MessageFramer(this, bufferAllocator, statsTraceCtx);
this.headers = headers;
} else {
framer = new GetFramer(headers, statsTraceCtx);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,11 @@ protected interface Sink {
private boolean outboundClosed;
private boolean headersSent;

@SuppressWarnings("this-escape")
protected AbstractServerStream(
WritableBufferAllocator bufferAllocator, StatsTraceContext statsTraceCtx) {
this.statsTraceCtx = Preconditions.checkNotNull(statsTraceCtx, "statsTraceCtx");
framer = new MessageFramer(this, bufferAllocator, statsTraceCtx);
this.framer = new MessageFramer(this, bufferAllocator, statsTraceCtx);
}

@Override
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/io/grpc/internal/AbstractStream.java
Original file line number Diff line number Diff line change
Expand Up @@ -163,20 +163,21 @@ public abstract static class TransportState
@GuardedBy("onReadyLock")
private int onReadyThreshold;

@SuppressWarnings("this-escape")
protected TransportState(
int maxMessageSize,
StatsTraceContext statsTraceCtx,
TransportTracer transportTracer) {
this.statsTraceCtx = checkNotNull(statsTraceCtx, "statsTraceCtx");
this.transportTracer = checkNotNull(transportTracer, "transportTracer");
rawDeframer = new MessageDeframer(
this.rawDeframer = new MessageDeframer(
this,
Codec.Identity.NONE,
maxMessageSize,
statsTraceCtx,
transportTracer);
// TODO(#7168): use MigratingThreadDeframer when enabling retry doesn't break.
deframer = rawDeframer;
deframer = this.rawDeframer;
onReadyThreshold = DEFAULT_ONREADY_THRESHOLD;
}

Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/io/grpc/internal/JsonUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ private static int parseNanos(String value) throws ParseException {
return result;
}

private static final long NANOS_PER_SECOND = TimeUnit.SECONDS.toNanos(1);
private static final int NANOS_PER_SECOND = 1_000_000_000;

/**
* Copy of {@link com.google.protobuf.util.Durations#normalizedDuration}.
Expand All @@ -368,11 +368,11 @@ private static long normalizedDuration(long seconds, int nanos) {
nanos %= NANOS_PER_SECOND;
}
if (seconds > 0 && nanos < 0) {
nanos += NANOS_PER_SECOND; // no overflow since nanos is negative (and we're adding)
nanos += NANOS_PER_SECOND; // no overflow nanos is negative (and we're adding)
seconds--; // no overflow since seconds is positive (and we're decrementing)
}
if (seconds < 0 && nanos > 0) {
nanos -= NANOS_PER_SECOND; // no overflow since nanos is positive (and we're subtracting)
nanos -= NANOS_PER_SECOND; // no overflow nanos is positive (and we're subtracting)
seconds++; // no overflow since seconds is negative (and we're incrementing)
}
if (!durationIsValid(seconds, nanos)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import com.google.errorprone.annotations.concurrent.GuardedBy;
import io.grpc.ExperimentalApi;
import java.io.IOException;
import java.io.NotSerializableException;
import java.io.ObjectOutputStream;
import java.net.SocketAddress;
import javax.annotation.Nullable;

Expand All @@ -34,7 +36,11 @@ public final class AnonymousInProcessSocketAddress extends SocketAddress {

@Nullable
@GuardedBy("this")
private InProcessServer server;
private transient InProcessServer server;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Serializing this object won't work. I'd be tempted to make it fail explicitly.


private void writeObject(ObjectOutputStream out) throws IOException {
throw new NotSerializableException("AnonymousInProcessSocketAddress is not serializable");
}

/** Creates a new AnonymousInProcessSocketAddress. */
public AnonymousInProcessSocketAddress() { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public abstract class GrpcHttp2ConnectionHandler extends Http2ConnectionHandler
usingPre4_1_111_Netty = identifiedOldVersion;
}

@SuppressWarnings("this-escape")
protected GrpcHttp2ConnectionHandler(
ChannelPromise channelUnused,
Http2ConnectionDecoder decoder,
Expand Down
1 change: 1 addition & 0 deletions servlet/src/main/java/io/grpc/servlet/GrpcServlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
public class GrpcServlet extends HttpServlet {
private static final long serialVersionUID = 1L;

@SuppressWarnings("serial")
private final ServletAdapter servletAdapter;

GrpcServlet(ServletAdapter servletAdapter) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ public class ChildLbState {
private ConnectivityState currentState;
private SubchannelPicker currentPicker = new FixedResultPicker(PickResult.withNoResult());

@SuppressWarnings("this-escape")
public ChildLbState(Object key, LoadBalancer.Factory policyFactory) {
this.key = key;
this.lb = policyFactory.newLoadBalancer(createChildHelper());
Expand Down
63 changes: 37 additions & 26 deletions xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ public boolean shouldAccept(Runnable command) {
protected final Queue<LrsRpcCall> loadReportCalls = new ArrayDeque<>();
protected final AtomicBoolean adsEnded = new AtomicBoolean(true);
protected final AtomicBoolean lrsEnded = new AtomicBoolean(true);
private final MessageFactory mf = createMessageFactory();
protected MessageFactory mf;

private static final long TIME_INCREMENT = TimeUnit.SECONDS.toNanos(1);
/** Fake time provider increments time TIME_INCREMENT each call. */
Expand All @@ -234,37 +234,22 @@ public long currentTimeNanos() {

private static final int VHOST_SIZE = 2;
// LDS test resources.
private final Any testListenerVhosts = Any.pack(mf.buildListenerWithApiListener(LDS_RESOURCE,
mf.buildRouteConfiguration("do not care", mf.buildOpaqueVirtualHosts(VHOST_SIZE))));
private final Any testListenerRds =
Any.pack(mf.buildListenerWithApiListenerForRds(LDS_RESOURCE, RDS_RESOURCE));
private Any testListenerVhosts;
private Any testListenerRds;

// RDS test resources.
private final Any testRouteConfig =
Any.pack(mf.buildRouteConfiguration(RDS_RESOURCE, mf.buildOpaqueVirtualHosts(VHOST_SIZE)));
private Any testRouteConfig;

// CDS test resources.
private final Any testClusterRoundRobin =
Any.pack(mf.buildEdsCluster(CDS_RESOURCE, null, "round_robin", null,
null, false, null, "envoy.transport_sockets.tls", null, null
));
private Any testClusterRoundRobin;

// EDS test resources.
private final Message lbEndpointHealthy =
mf.buildLocalityLbEndpoints("region1", "zone1", "subzone1",
mf.buildLbEndpoint("192.168.0.1", 8080, "healthy", 2, "endpoint-host-name"), 1, 0);
private Message lbEndpointHealthy;
// Locality with 0 endpoints
private final Message lbEndpointEmpty =
mf.buildLocalityLbEndpoints("region3", "zone3", "subzone3",
ImmutableList.<Message>of(), 2, 1);
private Message lbEndpointEmpty;
// Locality with 0-weight endpoint
private final Message lbEndpointZeroWeight =
mf.buildLocalityLbEndpoints("region4", "zone4", "subzone4",
mf.buildLbEndpoint("192.168.142.5", 80, "unknown", 5, "endpoint-host-name"), 0, 2);
private final Any testClusterLoadAssignment = Any.pack(mf.buildClusterLoadAssignment(EDS_RESOURCE,
ImmutableList.of(lbEndpointHealthy, lbEndpointEmpty, lbEndpointZeroWeight),
ImmutableList.of(mf.buildDropOverload("lb", 200), mf.buildDropOverload("throttle", 1000))));

private Message lbEndpointZeroWeight;
private Any testClusterLoadAssignment;
@Captor
private ArgumentCaptor<LdsUpdate> ldsUpdateCaptor;
@Captor
Expand Down Expand Up @@ -302,8 +287,8 @@ public long currentTimeNanos() {
private boolean originalEnableLeastRequest;
private Server xdsServer;
private final String serverName = InProcessServerBuilder.generateName();
private final BindableService adsService = createAdsService();
private final BindableService lrsService = createLrsService();
private BindableService adsService;
private BindableService lrsService;

private XdsTransportFactory xdsTransportFactory = new XdsTransportFactory() {
@Override
Expand Down Expand Up @@ -331,6 +316,32 @@ public XdsTransport create(ServerInfo serverInfo) {

@Before
public void setUp() throws IOException {
mf = createMessageFactory();
testListenerVhosts = Any.pack(mf.buildListenerWithApiListener(LDS_RESOURCE,
mf.buildRouteConfiguration("do not care", mf.buildOpaqueVirtualHosts(VHOST_SIZE))));
testListenerRds =
Any.pack(mf.buildListenerWithApiListenerForRds(LDS_RESOURCE, RDS_RESOURCE));
testRouteConfig =
Any.pack(mf.buildRouteConfiguration(RDS_RESOURCE, mf.buildOpaqueVirtualHosts(VHOST_SIZE)));
testClusterRoundRobin =
Any.pack(mf.buildEdsCluster(CDS_RESOURCE, null, "round_robin", null,
null, false, null, "envoy.transport_sockets.tls", null, null
));
lbEndpointHealthy =
mf.buildLocalityLbEndpoints("region1", "zone1", "subzone1",
mf.buildLbEndpoint("192.168.0.1", 8080, "healthy", 2, "endpoint-host-name"), 1, 0);
lbEndpointEmpty =
mf.buildLocalityLbEndpoints("region3", "zone3", "subzone3",
ImmutableList.<Message>of(), 2, 1);
lbEndpointZeroWeight =
mf.buildLocalityLbEndpoints("region4", "zone4", "subzone4",
mf.buildLbEndpoint("192.168.142.5", 80, "unknown", 5, "endpoint-host-name"), 0, 2);
testClusterLoadAssignment = Any.pack(mf.buildClusterLoadAssignment(EDS_RESOURCE,
ImmutableList.of(lbEndpointHealthy, lbEndpointEmpty, lbEndpointZeroWeight),
ImmutableList.of(mf.buildDropOverload("lb", 200), mf.buildDropOverload("throttle", 1000))));
adsService = createAdsService();
lrsService = createLrsService();

when(backoffPolicyProvider.get()).thenReturn(backoffPolicy1, backoffPolicy2);
when(backoffPolicy1.nextBackoffNanos()).thenReturn(10L, 100L);
when(backoffPolicy2.nextBackoffNanos()).thenReturn(20L, 200L);
Expand Down