diff --git a/core/src/main/java/io/grpc/internal/RetriableStream.java b/core/src/main/java/io/grpc/internal/RetriableStream.java index 9ca2280f6b1e..7fae3f8ae39d 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 @@ abstract class RetriableStream implements ClientStream { this.throttle = throttle; } + @SuppressWarnings("GuardedBy") @Nullable // null if already committed @CheckReturnValue private Runnable commit(final Substream winningSubstream) { @@ -138,6 +139,8 @@ private Runnable commit(final Substream winningSubstream) { 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 @@ private Runnable commit(final Substream winningSubstream) { // 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 @@ public void runWith(Substream substream) { drain(substream); } + @SuppressWarnings("GuardedBy") private void pushbackHedging(@Nullable Integer delayMillis) { if (delayMillis == null) { return; @@ -356,6 +362,8 @@ private void pushbackHedging(@Nullable Integer delayMillis) { 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 @@ private final class HedgingRunnable implements Runnable { 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 @@ public void run() { 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 @@ private boolean hasPotentialHedging(State state) { && !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 1e946ead1fce..fab249e656eb 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 @@ public void run() { 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 b7f298a2fa9f..44a86b2a71b3 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 @@ public TransportState( 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 @@ private void onEndOfStream() { } } + @SuppressWarnings("GuardedBy") @GuardedBy("lock") private void cancel(Status reason, boolean stopDelivery, Metadata trailers) { if (cancelSent) { @@ -373,6 +377,8 @@ private void cancel(Status reason, boolean stopDelivery, Metadata trailers) { 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 @@ private void sendBuffer(Buffer buffer, boolean endOfStream, boolean flush) { } } + @SuppressWarnings("GuardedBy") @GuardedBy("lock") private void streamReady(Metadata metadata, String path) { requestHeaders = @@ -416,6 +423,8 @@ private void streamReady(Metadata metadata, String path) { 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 12181c944c7e..b238b9237d68 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 @@ void streamReadyToStart(OkHttpClientStream clientStream) { } } + @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 @@ public void run() { /** * 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 @@ public void data(boolean inFinished, int streamId, BufferedSource in, int length 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 @@ public void data(boolean inFinished, int streamId, BufferedSource in, int length /** * Handle HTTP2 HEADER and CONTINUATION frames. */ + @SuppressWarnings("GuardedBy") @Override public void headers(boolean outFinished, boolean inFinished, @@ -1186,6 +1193,8 @@ public void headers(boolean outFinished, 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) {