Skip to content

Conversation

@Nepomuk5665
Copy link

Summary

Fix a bug in Topology.closeCheckedOutConnections() where only the first server's checked-out connections were being closed.

The Bug

The method had a premature return statement inside the for loop:

closeCheckedOutConnections() {
  for (const server of this.s.servers.values()) {
    return server.closeCheckedOutConnections();  // Returns on first iteration!
  }
}

This caused the method to:

  1. Get the first server from the servers Map
  2. Call closeCheckedOutConnections() on it
  3. Immediately return, skipping all remaining servers

The Fix

Remove the return statement so all servers are iterated:

closeCheckedOutConnections() {
  for (const server of this.s.servers.values()) {
    server.closeCheckedOutConnections();
  }
}

Impact

This bug affects multi-server topologies (replica sets, sharded clusters) where only the first server's connections would be properly closed when MongoClient.close() is called. This could lead to:

  • Connection leaks on servers other than the first one
  • Resources not being properly cleaned up during client shutdown

The method had a premature 'return' inside the for loop, which caused it
to only close connections on the first server and exit immediately. This
fix removes the return statement so all servers have their checked-out
connections closed when MongoClient.close() is called.

This bug would affect multi-server topologies (replica sets, sharded
clusters) where only the first server's connections would be properly
closed.
@Nepomuk5665 Nepomuk5665 requested a review from a team as a code owner January 23, 2026 23:13
@tadjik1 tadjik1 self-assigned this Jan 26, 2026
@tadjik1 tadjik1 added Primary Review In Review with primary reviewer, not yet ready for team's eyes External Submission PR submitted from outside the team labels Jan 26, 2026
@tadjik1
Copy link
Member

tadjik1 commented Jan 26, 2026

Hi @Nepomuk5665 thanks so much for the work on this PR. We're currently reviewing this and will get back to you soon.

@tadjik1 tadjik1 changed the title fix: iterate all servers in closeCheckedOutConnections() fix(NODE-7412): iterate all servers in closeCheckedOutConnections() Jan 26, 2026
@tadjik1 tadjik1 added the tracked-in-jira Ticket filed in MongoDB's Jira system label Jan 26, 2026
@tadjik1 tadjik1 changed the title fix(NODE-7412): iterate all servers in closeCheckedOutConnections() fix(NODE-7411): iterate all servers in closeCheckedOutConnections() Jan 26, 2026
@tadjik1
Copy link
Member

tadjik1 commented Jan 27, 2026

@Nepomuk5665 fantastic job, thanks for pointing out this problem. Could you please modify the existing test inside test/integration/connection-monitoring-and-pooling/connection_pool.test.ts so that it validates the fix as well?

There are currently two issues with the test:

  • Metadata: The metadata for the existing ConnectionCheckedInEvent test is set to single. This needs to be either removed or updated to support multi-server deployments.
  • Validation: The test only executes an insert. We aren't validating that any operations or connections are being issued against secondaries. It would be better to explicitly set a readPreference to ensure multiple servers are exercised.

…deployments

- Remove 'single' topology restriction from metadata to support replicaset/sharded
- Use readPreference: 'secondaryPreferred' to exercise connections to secondaries
- Switch from insert to find operations to validate reads against secondaries
@Nepomuk5665
Copy link
Author

Thanks for the feedback @tadjik1! I've updated the test with the following changes:

  1. Metadata: Removed the topology: 'single' restriction so the test now runs on all topologies (single, replicaset, and sharded)

  2. Validation:

    • Added readPreference: 'secondaryPreferred' to the client configuration to ensure operations can be routed to secondaries when available
    • Changed from insert to find operations since reads respect readPreference while writes always go to primary
    • Updated the failpoint to block find commands instead of insert

This ensures that in multi-server deployments, the test will exercise connections to both primary and secondary servers, validating that the fix properly checks in connections across all servers on close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

External Submission PR submitted from outside the team Primary Review In Review with primary reviewer, not yet ready for team's eyes tracked-in-jira Ticket filed in MongoDB's Jira system

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants