From 9554e0d44a5682aec602fa99a2fa7ef4f4edebfc Mon Sep 17 00:00:00 2001 From: themechbro Date: Wed, 18 Mar 2026 22:51:44 +0530 Subject: [PATCH 1/4] core: fix false-positive orphan warning in ManagedChannelOrphanWrapper Add a reachability fence in shutdown() and shutdownNow() to ensure the wrapper is not garbage collected while shutdown logic is executing. This prevents a race condition when using directExecutor() where a warning could be logged despite a proper shutdown. Fixes #12641 --- .../internal/ManagedChannelOrphanWrapper.java | 17 ++++++-- .../ManagedChannelOrphanWrapperTest.java | 39 +++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelOrphanWrapper.java b/core/src/main/java/io/grpc/internal/ManagedChannelOrphanWrapper.java index eac9b64d9db..c7f722a9499 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelOrphanWrapper.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelOrphanWrapper.java @@ -62,14 +62,24 @@ final class ManagedChannelOrphanWrapper extends ForwardingManagedChannel { @Override public ManagedChannel shutdown() { + ManagedChannel result = super.shutdown(); phantom.clearSafely(); - return super.shutdown(); + // This dummy check prevents the JIT from collecting 'this' too early + if (this.getClass() == null) { + throw new AssertionError(); + } + return result; } @Override public ManagedChannel shutdownNow() { + ManagedChannel result = super.shutdownNow(); phantom.clearSafely(); - return super.shutdownNow(); + // This dummy check prevents the JIT from collecting 'this' too early + if (this.getClass() == null) { + throw new AssertionError(); + } + return result; } @VisibleForTesting @@ -151,8 +161,9 @@ static int cleanQueue(ReferenceQueue refqueue) { int orphanedChannels = 0; while ((ref = (ManagedChannelReference) refqueue.poll()) != null) { RuntimeException maybeAllocationSite = ref.allocationSite.get(); + boolean wasShutdown = ref.shutdown.get(); ref.clearInternal(); // technically the reference is gone already. - if (!ref.shutdown.get()) { + if (!wasShutdown) { orphanedChannels++; Level level = Level.SEVERE; if (logger.isLoggable(level)) { diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java index 5ae97c69211..7ddab5b1ceb 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java @@ -101,6 +101,45 @@ public boolean isDone() { } } + @Test + public void shutdownNow_withDelegateStillReferenced_doesNotLogWarning() { + ManagedChannel mc = new TestManagedChannel(); + final ReferenceQueue refqueue = new ReferenceQueue<>(); + ConcurrentMap refs = + new ConcurrentHashMap<>(); + + ManagedChannelOrphanWrapper wrapper = new ManagedChannelOrphanWrapper(mc, refqueue, refs); + WeakReference wrapperWeakRef = new WeakReference<>(wrapper); + + final List records = new ArrayList<>(); + Logger orphanLogger = Logger.getLogger(ManagedChannelOrphanWrapper.class.getName()); + Filter oldFilter = orphanLogger.getFilter(); + orphanLogger.setFilter(new Filter() { + @Override + public boolean isLoggable(LogRecord record) { + synchronized (records) { + records.add(record); + } + return false; + } + }); + + try { + wrapper.shutdownNow(); + wrapper = null; + + // Wait for the WRAPPER itself to be garbage collected + GcFinalization.awaitClear(wrapperWeakRef); + ManagedChannelReference.cleanQueue(refqueue); + + synchronized (records) { + assertEquals("Warning was logged even though shutdownNow() was called!", 0, records.size()); + } + } finally { + orphanLogger.setFilter(oldFilter); + } + } + @Test public void refCycleIsGCed() { ReferenceQueue refqueue = From ffe4072622ca1e4b66f0ea6ceaac19984d9a9b4b Mon Sep 17 00:00:00 2001 From: themechbro Date: Wed, 18 Mar 2026 23:16:15 +0530 Subject: [PATCH 2/4] test: add coverage for standard shutdown() method --- .../java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java index 7ddab5b1ceb..a285f0b1ccb 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java @@ -125,7 +125,7 @@ public boolean isLoggable(LogRecord record) { }); try { - wrapper.shutdownNow(); + wrapper.shutdown(); wrapper = null; // Wait for the WRAPPER itself to be garbage collected From f81fc0b7004b62691503cdb8b6f6e6d08ae38b2e Mon Sep 17 00:00:00 2001 From: themechbro Date: Wed, 18 Mar 2026 23:40:12 +0530 Subject: [PATCH 3/4] test: add coverage for orphaned channel branch --- .../ManagedChannelOrphanWrapperTest.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java index a285f0b1ccb..a0f6063433b 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java @@ -140,6 +140,30 @@ public boolean isLoggable(LogRecord record) { } } + @Test + public void orphanedChannel_triggerWarningAndCoverage() { + ManagedChannel mc = new TestManagedChannel(); + final ReferenceQueue refqueue = new ReferenceQueue<>(); + ConcurrentMap refs = + new ConcurrentHashMap<>(); + + // Create the wrapper but NEVER call shutdown + ManagedChannelOrphanWrapper wrapper = new ManagedChannelOrphanWrapper(mc, refqueue, refs); + wrapper = null; // Make it eligible for GC + + // Trigger GC and clean the queue to hit the !wasShutdown branch + final AtomicInteger numOrphans = new AtomicInteger(); + GcFinalization.awaitDone(new FinalizationPredicate() { + @Override + public boolean isDone() { + numOrphans.getAndAdd(ManagedChannelReference.cleanQueue(refqueue)); + return numOrphans.get() > 0; + } + }); + + assertEquals(1, numOrphans.get()); + } + @Test public void refCycleIsGCed() { ReferenceQueue refqueue = From 46a6cdf26356c12bee12856f0a73e6339b365060 Mon Sep 17 00:00:00 2001 From: themechbro Date: Wed, 18 Mar 2026 23:54:32 +0530 Subject: [PATCH 4/4] test: suppress unused variable warning in orphan test --- .../java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java index a0f6063433b..aad002b27ce 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java @@ -148,6 +148,7 @@ public void orphanedChannel_triggerWarningAndCoverage() { new ConcurrentHashMap<>(); // Create the wrapper but NEVER call shutdown + @SuppressWarnings("UnusedVariable") ManagedChannelOrphanWrapper wrapper = new ManagedChannelOrphanWrapper(mc, refqueue, refs); wrapper = null; // Make it eligible for GC