Fix AbstractWriteSearchIndexOperation executeAsync releasing connections #1812
Conversation
Update SyncMongoClient to assert connections are released JAVA-5972
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a connection leak issue in the AbstractWriteSearchIndexOperation's executeAsync method and updates the reactive streams test client to properly assert connections are released. The main changes involve modifying SyncMongoClient to track active connections and updating test files to use the updated constructor pattern.
- Adds connection pooling monitoring to SyncMongoClient to track active connections and assert they are properly released when the client is closed
- Fixes a variable naming bug in AbstractWriteSearchIndexOperation.executeAsync that was preventing proper connection cleanup
- Updates all test files to use the new SyncMongoClient constructor pattern that accepts MongoClientSettings directly
Reviewed Changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| driver-core/src/main/com/mongodb/internal/operation/AbstractWriteSearchIndexOperation.java | Fixes variable name bug in executeAsync method |
| driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/syncadapter/SyncMongoClient.java | Adds connection pool monitoring and assertion logic |
| Multiple test files | Updates SyncMongoClient instantiation to use new constructor pattern |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...reams/src/test/functional/com/mongodb/reactivestreams/client/AsyncTransportSettingsTest.java
Outdated
Show resolved
Hide resolved
...reams/src/test/functional/com/mongodb/reactivestreams/client/AsyncTransportSettingsTest.java
Outdated
Show resolved
Hide resolved
|
Looks like there may be more leaks - will review and try to identify the causes. |
| try { | ||
| swallowOrThrow(commandExecutionError); | ||
| callback.onResult(result, null); | ||
| cb.onResult(result, null); |
There was a problem hiding this comment.
This the reason for the unreleased connection - it used the original and not decorated callback that handles the releasing.
The rest of the PR is updating the test suite to ensure there are no future regressions. The core is the SyncMongoClient close method that now has a listener that counts the connections out and in. It also asserts that all connections are released.
| AsyncCallbackSupplier<R> curriedFunction = c -> function.apply(resource, c); | ||
| curriedFunction.whenComplete(resource::release).get(errorHandlingCallback); | ||
| } catch (Exception e) { | ||
| if (resource.getCount() > 0) { |
There was a problem hiding this comment.
This more closely mirrors the SyncOperationHelper and will ensure any errors thrown outside of the callback also release the resource.
…ons (mongodb#1812) Update SyncMongoClient to assert connections are released Ensure errors thrown outside of the callback also release the resource in AsyncOperationHelper JAVA-5972
…ons (mongodb#1812) Cherry picked from main to 5.5.x Ensure errors thrown outside of the callback also release the resource in AsyncOperationHelper JAVA-5972
2 participants
Update SyncMongoClient to assert connections are released
JAVA-5972