Skip to content

Commit cab37d0

Browse files
committed
Improve readability of NettyStreamCloseFutureListenerTest
Extract the GC reachability probe into a named assertRefUnreachable helper that documents the WeakReference-as-reachability-probe idiom, extract the accept plumbing into openAndAcceptConnection, and add explanatory comments tying the footprint assertion back to JAVA-6250. Test-only, no behaviour change. JAVA-6250
1 parent cc27b4c commit cab37d0

1 file changed

Lines changed: 52 additions & 21 deletions

File tree

driver-core/src/test/unit/com/mongodb/internal/connection/netty/NettyStreamCloseFutureListenerTest.java

Lines changed: 52 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,14 @@ public void setUp() throws Exception {
7777
@AfterEach
7878
public void tearDown() throws Exception {
7979
stream.close();
80-
eventLoopGroup.shutdownGracefully();
80+
eventLoopGroup.shutdownGracefully().syncUninterruptibly();
8181
serverSocket.close();
8282
}
8383

8484
@Test
8585
@DisplayName("open handler should not remain strongly reachable from the open channel after the open completes")
8686
public void shouldReleaseOpenHandlerAfterOpenCompletesWhileChannelRemainsOpen() throws Exception {
87+
// Open a connection with a handler, and keep only a WeakReference to that handler
8788
CountDownLatch opened = new CountDownLatch(1);
8889
AsyncCompletionHandler<Void> handler = new AsyncCompletionHandler<Void>() {
8990
@Override
@@ -96,38 +97,29 @@ public void failed(final Throwable t) {
9697
opened.countDown();
9798
}
9899
};
99-
WeakReference<AsyncCompletionHandler<Void>> canary = new WeakReference<AsyncCompletionHandler<Void>>(handler);
100+
WeakReference<AsyncCompletionHandler<Void>> canary = new WeakReference<>(handler);
100101

101102
stream.openAsync(OPERATION_CONTEXT, handler);
102103
assertTrue(opened.await(10, TimeUnit.SECONDS), "open did not complete");
103104

105+
// Nullify the test's own reference so the driver is the only thing that could still retain the handler.
106+
// Note: the channel is deliberately left open (never closed) for the rest of the test, so a positive
107+
// result proves the handler is released while the channel is alive.
104108
handler = null;
105-
for (int i = 0; i < 10 && canary.get() != null; i++) {
106-
System.gc();
107-
Thread.sleep(100);
108-
}
109-
assertNull(canary.get(),
109+
110+
// The channel's closeFuture listener must not keep the one-shot open handler alive,
111+
// so with no strong reference remaining the handler must be garbage-collectable.
112+
assertRefUnreachable(canary,
110113
"the connection-open AsyncCompletionHandler must not stay strongly reachable from the open "
111114
+ "channel (closeFuture listener) after the open has completed");
112115
}
113116

114117
@Test
115118
@DisplayName("pending read should be failed when the channel is closed")
116119
public void shouldFailPendingReadWhenChannelIsClosed() throws Exception {
117-
AtomicReference<Socket> acceptedSocket = new AtomicReference<>();
118-
Thread acceptor = new Thread(() -> {
119-
try {
120-
acceptedSocket.set(serverSocket.accept());
121-
} catch (IOException ignored) {
122-
// the assertions below fail if nothing was accepted
123-
}
124-
});
125-
acceptor.start();
126-
127-
stream.open(OPERATION_CONTEXT);
128-
acceptor.join(TimeUnit.SECONDS.toMillis(10));
129-
assertNotNull(acceptedSocket.get(), "the server never accepted the connection");
120+
Socket acceptedSocket = openAndAcceptConnection();
130121

122+
// Create a read that 127.0.0.1 will never satisfy, then close the connection from the server side
131123
CountDownLatch readCompleted = new CountDownLatch(1);
132124
AtomicReference<Throwable> readFailure = new AtomicReference<>();
133125
stream.readAsync(4, OPERATION_CONTEXT, new AsyncCompletionHandler<ByteBuf>() {
@@ -143,12 +135,51 @@ public void failed(final Throwable t) {
143135
}
144136
});
145137

146-
acceptedSocket.get().close();
138+
acceptedSocket.close();
147139

140+
// Assert the closeFuture listener fires and fails the pending read with the expected IOException
148141
assertTrue(readCompleted.await(10, TimeUnit.SECONDS), "the pending read never completed");
149142
Throwable failure = readFailure.get();
150143
assertNotNull(failure, "the pending read completed successfully instead of failing");
151144
assertInstanceOf(IOException.class, failure);
152145
assertEquals("The connection to the server was closed", failure.getMessage());
153146
}
147+
148+
/**
149+
* Asserts that the referent of {@code ref} is no longer strongly reachable, i.e. no strong reference to it
150+
* remains and it is therefore garbage-collectable.
151+
* A {@link WeakReference} is a reachability probe: the garbage collector clears it as soon as its referent is
152+
* no longer strongly reachable. {@link System#gc()} is only a hint, so we nudge it a few times and give the
153+
* collector a moment before checking - if the referent is genuinely unreachable the reference clears quickly.
154+
*/
155+
@SuppressWarnings("BusyWait")
156+
private static void assertRefUnreachable(final WeakReference<?> ref, final String message) throws InterruptedException {
157+
long deadlineNanos = System.nanoTime() + TimeUnit.SECONDS.toNanos(10);
158+
while (ref.get() != null && System.nanoTime() < deadlineNanos) {
159+
System.gc();
160+
Thread.sleep(100);
161+
}
162+
assertNull(ref.get(), message);
163+
}
164+
165+
/**
166+
* Opens {@link #stream} against the local {@link #serverSocket} and returns the server side of the accepted
167+
* connection, so the test can later close it to simulate the server dropping the connection.
168+
*/
169+
private Socket openAndAcceptConnection() throws Exception {
170+
AtomicReference<Socket> acceptedSocket = new AtomicReference<>();
171+
Thread acceptor = new Thread(() -> {
172+
try {
173+
acceptedSocket.set(serverSocket.accept());
174+
} catch (IOException ignored) {
175+
// the assertion below fails if nothing was accepted
176+
}
177+
});
178+
acceptor.start();
179+
180+
stream.open(OPERATION_CONTEXT);
181+
acceptor.join(TimeUnit.SECONDS.toMillis(10));
182+
assertNotNull(acceptedSocket.get(), "the server never accepted the connection");
183+
return acceptedSocket.get();
184+
}
154185
}

0 commit comments

Comments
 (0)