RUBY-3669 Fix memory leak when iterating cursors#2995
RUBY-3669 Fix memory leak when iterating cursors#2995comandeo-mongo merged 1 commit intomongodb:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a cursor-iteration memory leak where implicit sessions could retain a cloned Mongo::Client via cursor finalizer state, causing one client object to be retained per iteration when batch_size < limit.
Changes:
- Add an MRI-only integration regression spec to detect leaked
Mongo::Clientinstances during cursor iteration. - Avoid cloning the client for implicit sessions and retain the cluster separately for session lifecycle operations.
- Proactively end implicit sessions when queued cursor kill specs are already stale (cursor already closed).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| spec/integration/cursor_memory_leak_spec.rb | Adds a regression test that counts Mongo::Client objects across repeated cursor iterations. |
| lib/mongo/session.rb | Prevents implicit-session client cloning and clears client references on session end to break retention chains. |
| lib/mongo/cluster/reapers/cursor_reaper.rb | Ends implicit sessions early when dequeued kill specs refer to already-closed cursors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
|
||
| require 'spec_helper' | ||
|
|
||
| describe 'Cursor memory leak - RUBY-3669' do |
There was a problem hiding this comment.
The spec description references RUBY-3669, but the PR title/description is RUBY-3369. Please correct the ticket number here so the regression test is discoverable and matches the tracking issue.
There was a problem hiding this comment.
Fair point -- I think the PR is probably misnamed?
| GC.start | ||
| GC.start | ||
| GC.start | ||
| sleep Mongo::Cluster::CursorReaper::FREQUENCY * 2 + 1 | ||
|
|
There was a problem hiding this comment.
This test relies on sleep Mongo::Cluster::CursorReaper::FREQUENCY * 2 + 1 (and a second identical sleep later), which adds several seconds to the suite and can still be timing-sensitive. Consider triggering the periodic executor directly (as done in spec/integration/cursor_reaping_spec.rb) to make the test faster and more deterministic.
| ensure | ||
| @server_session = nil | ||
| @ended = true | ||
| @client = nil | ||
| end |
There was a problem hiding this comment.
end_session now sets @client = nil, which makes the public Session#client reader return nil after a session is ended (and can turn some accidental post-end method calls into NoMethodError rather than SessionEnded). If clearing the client reference is only needed to break implicit-session leaks, consider limiting this to implicit sessions or keeping a separate non-leaking reference for #client and updating the documentation accordingly.
|
|
||
| require 'spec_helper' | ||
|
|
||
| describe 'Cursor memory leak - RUBY-3669' do |
There was a problem hiding this comment.
Fair point -- I think the PR is probably misnamed?
comandeo-mongo
changed the title
Iterating a cursor with batch_size < limit leaked a
Mongo::Clientobject per iteration because implicit sessions were cloning the client.