Skip to content

Commit 2e3ad1d

Browse files
authored
core: prevent data race in ManagedChannelOrphanWrapper (#6854)
Data race was detected internally when ManagedChannelOrphanWrapper.shutdown() was called concurrently: WARNING: ThreadSanitizer: data race (pid=5009) Write of size 8 at 0x7fd2f7f37530 by thread T49: #0 java.lang.ref.Reference.clear()V Reference.java:265 #1 io.grpc.internal.ManagedChannelOrphanWrapper$ManagedChannelReference.clearInternal()V ManagedChannelOrphanWrapper.java:118 #2 io.grpc.internal.ManagedChannelOrphanWrapper$ManagedChannelReference.clear()V ManagedChannelOrphanWrapper.java:110 #3 io.grpc.internal.ManagedChannelOrphanWrapper.shutdown()Lio/grpc/ManagedChannel; ManagedChannelOrphanWrapper.java:58 (stacktrace redacted) Previous write of size 8 at 0x7fd2f7f37530 by thread T45 (mutexes: write M267260296638793720, write M267541771615505864, write M267823246592216728, write M267260296898451984, write M267541771875162784, write M267823246851873416): #0 java.lang.ref.Reference.clear()V Reference.java:265 #1 io.grpc.internal.ManagedChannelOrphanWrapper$ManagedChannelReference.clearInternal()V ManagedChannelOrphanWrapper.java:118 #2 io.grpc.internal.ManagedChannelOrphanWrapper$ManagedChannelReference.clear()V ManagedChannelOrphanWrapper.java:110 #3 io.grpc.internal.ManagedChannelOrphanWrapper.shutdown()Lio/grpc/ManagedChannel; ManagedChannelOrphanWrapper.java:58 (stacktrace redacted)
1 parent 0ba9d3e commit 2e3ad1d

File tree

1 file changed

+14
-6
lines changed

1 file changed

+14
-6
lines changed

core/src/main/java/io/grpc/internal/ManagedChannelOrphanWrapper.java

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.lang.ref.WeakReference;
2525
import java.util.concurrent.ConcurrentHashMap;
2626
import java.util.concurrent.ConcurrentMap;
27+
import java.util.concurrent.atomic.AtomicBoolean;
2728
import java.util.logging.Level;
2829
import java.util.logging.LogRecord;
2930
import java.util.logging.Logger;
@@ -54,15 +55,13 @@ final class ManagedChannelOrphanWrapper extends ForwardingManagedChannel {
5455

5556
@Override
5657
public ManagedChannel shutdown() {
57-
phantom.shutdown = true;
58-
phantom.clear();
58+
phantom.clearSafely();
5959
return super.shutdown();
6060
}
6161

6262
@Override
6363
public ManagedChannel shutdownNow() {
64-
phantom.shutdown = true;
65-
phantom.clear();
64+
phantom.clearSafely();
6665
return super.shutdownNow();
6766
}
6867

@@ -81,7 +80,7 @@ static final class ManagedChannelReference extends WeakReference<ManagedChannelO
8180

8281
private final String channelStr;
8382
private final Reference<RuntimeException> allocationSite;
84-
private volatile boolean shutdown;
83+
private final AtomicBoolean shutdown = new AtomicBoolean();
8584

8685
ManagedChannelReference(
8786
ManagedChannelOrphanWrapper orphanable,
@@ -113,6 +112,15 @@ public void clear() {
113112
cleanQueue(refqueue);
114113
}
115114

115+
/**
116+
* Safe to call concurrently.
117+
*/
118+
private void clearSafely() {
119+
if (!shutdown.getAndSet(true)) {
120+
clear();
121+
}
122+
}
123+
116124
// avoid reentrancy
117125
private void clearInternal() {
118126
super.clear();
@@ -135,7 +143,7 @@ static int cleanQueue(ReferenceQueue<ManagedChannelOrphanWrapper> refqueue) {
135143
while ((ref = (ManagedChannelReference) refqueue.poll()) != null) {
136144
RuntimeException maybeAllocationSite = ref.allocationSite.get();
137145
ref.clearInternal(); // technically the reference is gone already.
138-
if (!ref.shutdown) {
146+
if (!ref.shutdown.get()) {
139147
orphanedChannels++;
140148
Level level = Level.SEVERE;
141149
if (logger.isLoggable(level)) {

0 commit comments

Comments
 (0)