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..aad002b27ce 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelOrphanWrapperTest.java @@ -101,6 +101,70 @@ 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.shutdown(); + 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 orphanedChannel_triggerWarningAndCoverage() { + ManagedChannel mc = new TestManagedChannel(); + final ReferenceQueue refqueue = new ReferenceQueue<>(); + ConcurrentMap refs = + 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 + + // 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 =