all: Add suppressions for GuardedBy violations
This supports releasing a new version of GuardedBy which finds more mistakes than it used to.
Filed #6578 to try to clean up the suppressions.
diff --git a/core/src/main/java/io/grpc/internal/RetriableStream.java b/core/src/main/java/io/grpc/internal/RetriableStream.java
index 9ca2280..7fae3f8 100644
--- a/core/src/main/java/io/grpc/internal/RetriableStream.java
+++ b/core/src/main/java/io/grpc/internal/RetriableStream.java
@@ -121,6 +121,7 @@
this.throttle = throttle;
}
+ @SuppressWarnings("GuardedBy")
@Nullable // null if already committed
@CheckReturnValue
private Runnable commit(final Substream winningSubstream) {
@@ -138,6 +139,8 @@
final Future<?> retryFuture;
if (scheduledRetry != null) {
+ // TODO(b/145386688): This access should be guarded by 'this.scheduledRetry.lock'; instead
+ // found: 'this.lock'
retryFuture = scheduledRetry.markCancelled();
scheduledRetry = null;
} else {
@@ -146,6 +149,8 @@
// cancel the scheduled hedging if it is scheduled prior to the commitment
final Future<?> hedgingFuture;
if (scheduledHedging != null) {
+ // TODO(b/145386688): This access should be guarded by 'this.scheduledHedging.lock'; instead
+ // found: 'this.lock'
hedgingFuture = scheduledHedging.markCancelled();
scheduledHedging = null;
} else {
@@ -338,6 +343,7 @@
drain(substream);
}
+ @SuppressWarnings("GuardedBy")
private void pushbackHedging(@Nullable Integer delayMillis) {
if (delayMillis == null) {
return;
@@ -356,6 +362,8 @@
return;
}
+ // TODO(b/145386688): This access should be guarded by 'this.scheduledHedging.lock'; instead
+ // found: 'this.lock'
futureToBeCancelled = scheduledHedging.markCancelled();
scheduledHedging = future = new FutureCanceller(lock);
}
@@ -381,6 +389,7 @@
public void run() {
callExecutor.execute(
new Runnable() {
+ @SuppressWarnings("GuardedBy")
@Override
public void run() {
// It's safe to read state.hedgingAttemptCount here.
@@ -392,6 +401,9 @@
FutureCanceller future = null;
synchronized (lock) {
+ // TODO(b/145386688): This access should be guarded by
+ // 'HedgingRunnable.this.scheduledHedgingRef.lock'; instead found:
+ // 'RetriableStream.this.lock'
if (scheduledHedgingRef.isCancelled()) {
cancelled = true;
} else {
@@ -695,10 +707,13 @@
&& !state.hedgingFrozen;
}
+ @SuppressWarnings("GuardedBy")
private void freezeHedging() {
Future<?> futureToBeCancelled = null;
synchronized (lock) {
if (scheduledHedging != null) {
+ // TODO(b/145386688): This access should be guarded by 'this.scheduledHedging.lock'; instead
+ // found: 'this.lock'
futureToBeCancelled = scheduledHedging.markCancelled();
scheduledHedging = null;
}
diff --git a/cronet/src/main/java/io/grpc/cronet/CronetClientTransport.java b/cronet/src/main/java/io/grpc/cronet/CronetClientTransport.java
index 1e946ea..fab249e 100644
--- a/cronet/src/main/java/io/grpc/cronet/CronetClientTransport.java
+++ b/cronet/src/main/java/io/grpc/cronet/CronetClientTransport.java
@@ -149,9 +149,12 @@
return new StartCallback().clientStream;
}
+ @SuppressWarnings("GuardedBy")
@GuardedBy("lock")
private void startStream(CronetClientStream stream) {
streams.add(stream);
+ // TODO(b/145386688): This access should be guarded by 'stream.transportState().lock'; instead
+ // found: 'this.lock'
stream.transportState().start(streamFactory);
}
diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientStream.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientStream.java
index b7f298a..44a86b2 100644
--- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientStream.java
+++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientStream.java
@@ -256,10 +256,13 @@
tag = PerfMark.createTag(methodName);
}
+ @SuppressWarnings("GuardedBy")
@GuardedBy("lock")
public void start(int streamId) {
checkState(id == ABSENT_ID, "the stream has been started with id %s", streamId);
id = streamId;
+ // TODO(b/145386688): This access should be guarded by 'OkHttpClientStream.this.state.lock';
+ // instead found: 'this.lock'
state.onStreamAllocated();
if (canStart) {
@@ -365,6 +368,7 @@
}
}
+ @SuppressWarnings("GuardedBy")
@GuardedBy("lock")
private void cancel(Status reason, boolean stopDelivery, Metadata trailers) {
if (cancelSent) {
@@ -373,6 +377,8 @@
cancelSent = true;
if (canStart) {
// stream is pending.
+ // TODO(b/145386688): This access should be guarded by 'this.transport.lock'; instead found:
+ // 'this.lock'
transport.removePendingStream(OkHttpClientStream.this);
// release holding data, so they can be GCed or returned to pool earlier.
requestHeaders = null;
@@ -406,6 +412,7 @@
}
}
+ @SuppressWarnings("GuardedBy")
@GuardedBy("lock")
private void streamReady(Metadata metadata, String path) {
requestHeaders =
@@ -416,6 +423,8 @@
userAgent,
useGet,
transport.isUsingPlaintext());
+ // TODO(b/145386688): This access should be guarded by 'this.transport.lock'; instead found:
+ // 'this.lock'
transport.streamReadyToStart(OkHttpClientStream.this);
}
diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java
index 12181c9..b238b92 100644
--- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java
+++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java
@@ -424,12 +424,15 @@
}
}
+ @SuppressWarnings("GuardedBy")
@GuardedBy("lock")
private void startStream(OkHttpClientStream stream) {
Preconditions.checkState(
stream.id() == OkHttpClientStream.ABSENT_ID, "StreamId already assigned");
streams.put(nextStreamId, stream);
setInUse(stream);
+ // TODO(b/145386688): This access should be guarded by 'stream.transportState().lock'; instead
+ // found: 'this.lock'
stream.transportState().start(nextStreamId);
// For unary and server streaming, there will be a data frame soon, no need to flush the header.
if ((stream.getType() != MethodType.UNARY && stream.getType() != MethodType.SERVER_STREAMING)
@@ -1111,6 +1114,7 @@
/**
* Handle an HTTP2 DATA frame.
*/
+ @SuppressWarnings("GuardedBy")
@Override
public void data(boolean inFinished, int streamId, BufferedSource in, int length)
throws IOException {
@@ -1136,6 +1140,8 @@
PerfMark.event("OkHttpClientTransport$ClientFrameHandler.data",
stream.transportState().tag());
synchronized (lock) {
+ // TODO(b/145386688): This access should be guarded by 'stream.transportState().lock';
+ // instead found: 'OkHttpClientTransport.this.lock'
stream.transportState().transportDataReceived(buf, inFinished);
}
}
@@ -1153,6 +1159,7 @@
/**
* Handle HTTP2 HEADER and CONTINUATION frames.
*/
+ @SuppressWarnings("GuardedBy")
@Override
public void headers(boolean outFinished,
boolean inFinished,
@@ -1186,6 +1193,8 @@
if (failedStatus == null) {
PerfMark.event("OkHttpClientTransport$ClientFrameHandler.headers",
stream.transportState().tag());
+ // TODO(b/145386688): This access should be guarded by 'stream.transportState().lock';
+ // instead found: 'OkHttpClientTransport.this.lock'
stream.transportState().transportHeadersReceived(headerBlock, inFinished);
} else {
if (!inFinished) {
