PR review updates

Author: rozza

config/spotbugs/exclude.xml

@@ -292,7 +292,7 @@
 
     <!-- DefaultServerMonitor -->
     <Match>
-        <class name="com.mongodb.internal.connection.DefaultServerMonitor" />
+        <Class name="com.mongodb.internal.connection.DefaultServerMonitor" />
         <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE" />
     </Match>
 

driver-core/src/main/com/mongodb/internal/connection/DefaultServerMonitor.java

@@ -340,7 +340,12 @@ private ServerDescription doHeartbeat(final ServerDescription currentServerDescr
         }
 
         private void logAndNotifyHeartbeatStarted(final boolean shouldStreamResponses) {
-            ConnectionDescription description = connection.getDescription();
+            ConnectionDescription description = withLock(lock, () -> {
+                if (connection != null) {
+                    return connection.getDescription();
+                }
+                return null;
+            });
             if (description != null) {
                 alreadyLoggedHeartBeatStarted = true;
                 logHeartbeatStarted(serverId, description, shouldStreamResponses);
@@ -357,20 +362,34 @@ private void logAndNotifyHeartbeatStarted(final boolean shouldStreamResponses) {
         private void logAndNotifyHeartbeatSucceeded(final boolean shouldStreamResponses, final BsonDocument helloResult) {
             alreadyLoggedHeartBeatStarted = false;
             long elapsedTimeNanos = getElapsedTimeNanos();
-            if (!shouldStreamResponses) {
-                roundTripTimeSampler.addSample(elapsedTimeNanos);
+
+            ConnectionDescription description = withLock(lock, () -> {
+                if (connection != null) {
+                    return connection.getDescription();
+                }
+                return null;
+            });
+            if (description != null) {
+                if (!shouldStreamResponses) {
+                    roundTripTimeSampler.addSample(elapsedTimeNanos);
+                }
+                logHeartbeatSucceeded(serverId, description, shouldStreamResponses, elapsedTimeNanos, helloResult);
+                serverMonitorListener.serverHeartbeatSucceeded(
+                        new ServerHeartbeatSucceededEvent(description.getConnectionId(), helloResult,
+                                elapsedTimeNanos, shouldStreamResponses));
             }
-            logHeartbeatSucceeded(serverId, connection.getDescription(), shouldStreamResponses, elapsedTimeNanos, helloResult);
-            serverMonitorListener.serverHeartbeatSucceeded(
-                    new ServerHeartbeatSucceededEvent(connection.getDescription().getConnectionId(), helloResult,
-                            elapsedTimeNanos, shouldStreamResponses));
         }
 
         private void logAndNotifyHeartbeatFailed(final boolean shouldStreamResponses, final Exception e) {
             alreadyLoggedHeartBeatStarted = false;
             long elapsedTimeNanos = getElapsedTimeNanos();
 
-            ConnectionDescription description = connection != null ? connection.getDescription() : null;
+            ConnectionDescription description = withLock(lock, () -> {
+                if (connection != null) {
+                    return connection.getDescription();
+                }
+                return null;
+            });
             if (description != null) {
                 logHeartbeatFailed(serverId, description, shouldStreamResponses, elapsedTimeNanos, e);
                 serverMonitorListener.serverHeartbeatFailed(
@@ -383,6 +402,7 @@ private void logAndNotifyHeartbeatFailed(final boolean shouldStreamResponses, fi
                 }
             }
         }
+
         private long getElapsedTimeNanos() {
             return System.nanoTime() - lookupStartTimeNanos;
         }

driver-core/src/test/functional/com/mongodb/internal/connection/ReplyHeaderTest.java

@@ -24,6 +24,8 @@
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
 
+import java.util.List;
+
 import static com.mongodb.connection.ConnectionDescription.getDefaultMaxMessageSize;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -45,12 +47,15 @@ void testParseReplyHeader(final int responseFlags) {
             outputBuffer.writeInt(4);
             outputBuffer.writeInt(1);
 
-            ByteBuf byteBuf = outputBuffer.getByteBuffers().get(0);
+            List<ByteBuf> byteBuffers = outputBuffer.getByteBuffers();
+            ByteBuf byteBuf = byteBuffers.get(0);
             ReplyHeader replyHeader = new ReplyHeader(byteBuf, new MessageHeader(byteBuf, getDefaultMaxMessageSize()));
 
             assertEquals(186, replyHeader.getMessageLength());
             assertEquals(45, replyHeader.getRequestId());
             assertEquals(23, replyHeader.getResponseTo());
+
+            byteBuffers.forEach(ByteBuf::release);
         }
     }
 

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

@@ -286,6 +286,7 @@ void closeDuringConnectionShouldNotLeakBuffers() throws Exception {
 
         // Verify no leaks by checking connection was properly closed
         monitor.getServerMonitor().join(5000);
+        assertFalse(monitor.getServerMonitor().isAlive());
     }
 
     @Test

driver-core/src/test/unit/com/mongodb/internal/session/ServerSessionPoolTest.java

@@ -56,6 +56,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.Mockito.mock;
@@ -157,7 +158,7 @@ void testPruneSessionsWhenGetting() {
         ServerSession sessionTwo = pool.get();
 
         assertNotEquals(sessionTwo, sessionOne);
-        // Note: Actual closed state verification depends on implementation details
+        assertTrue(sessionOne.isClosed());
     }
 
     @Test

driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/ClientSessionPublisherImpl.java

@@ -48,13 +48,13 @@
 
 final class ClientSessionPublisherImpl extends BaseClientSessionImpl implements ClientSession {
 
+    private final AtomicBoolean closed = new AtomicBoolean();
     private final MongoClientImpl mongoClient;
     private final OperationExecutor executor;
     private TransactionState transactionState = TransactionState.NONE;
     private boolean messageSentInCurrentTransaction;
     private boolean commitInProgress;
     private TransactionOptions transactionOptions;
-    private AtomicBoolean closed = new AtomicBoolean();
 
     ClientSessionPublisherImpl(final ServerSessionPool serverSessionPool, final MongoClientImpl mongoClient,
             final ClientSessionOptions options, final OperationExecutor executor) {

driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTest.java

@@ -312,7 +312,7 @@ public void cleanUp() {
             postCleanUp(testDef);
         }
         // Ask the JVM to run garbage collection.
-        // This should help with Nettys leak detection
+        // This should help with Netty's leak detection
         System.gc();
     }