Skip to content

CXX-3100 Check return when acquiring client from pool #1205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Sep 11, 2024

Conversation

Klizzardy
Copy link
Contributor

(This pull request fix one minor bug)
Symptom: When the uri parameter 'waitQueueTimeoutMS' was enabled and set to a smaller value (e.g. 100), the function 'mongoc_client_pool' may return a NULL client.
Solution: Add a check in the function 'pool::entry pool::acquire()', and throw an exception when correct client was not returned.

@kevinAlbs kevinAlbs self-requested a review September 9, 2024 18:49
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR @Klizzardy.

@Klizzardy
Copy link
Contributor Author

Thank you for your suggestions. They really helped me understand mongocxx better. I've made all the changes you recommended, and the updated code has been committed. @kevinAlbs

@Klizzardy
Copy link
Contributor Author

Thanks for your patience. I have updated the code based on your suggestions. If there are any other issues, feel free to let me know. @kevinAlbs

@kevinAlbs kevinAlbs changed the title Add a check on the client in the pool::acquire() function and throw e… CXX-3100 check for client in the pool::acquire() function and throw e… Sep 10, 2024
@kevinAlbs kevinAlbs changed the title CXX-3100 check for client in the pool::acquire() function and throw e… CXX-3100 Check return when acquiring client from pool Sep 10, 2024
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with additional minor requested changes.

@Klizzardy
Copy link
Contributor Author

All changes have been applied to the branch. If there are any other issues, please feel free to point them out. @kevinAlbs @eramongodb

Copy link
Contributor

@eramongodb eramongodb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some formatting changes remaining; otherwise, LGTM.

@kevinAlbs kevinAlbs merged commit fb04319 into mongodb:master Sep 11, 2024
71 of 79 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants