[SERVER-31138] ClusterClientCursorImpl isn't a safe raii type after releaseCursor Created: 18/Sep/17 Updated: 08/Jan/24 Resolved: 09/Aug/19 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Internal Code |
| Affects Version/s: | None |
| Fix Version/s: | 4.3.1 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Mira Carey | Assignee: | George Wangensteen |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | query-44-grooming | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||
| Operating System: | ALL | ||||||||
| Sprint: | Query 2019-07-29, Query 2019-08-12, Query 2019-08-26 | ||||||||
| Participants: | |||||||||
| Case: | (copied to CRM) | ||||||||
| Description |
|
The cluster client cursor impl returned by CursorEntry->releaseCursor() isn't safe to destroy. If it isn't put into a pinned cursor, it will invariant on destruction (due to getting cleaned up without actually being in the manager). We probably shouldn't have types like that (and mongod cursors don't work that way). |
| Comments |
| Comment by Githook User [ 09/Aug/19 ] |
|
Author: {'name': 'George Wangensteen', 'email': 'george.wangensteen@10gen.com'}Message: |
| Comment by Misha Tyulenev [ 03/Jul/19 ] |
|
charlie.swanson thanks, I agree that line 379 is problematic and releaseCursor should not be available except constructing the PinnedCursor. |
| Comment by Charlie Swanson [ 02/Jul/19 ] |
|
Came across this one again via this comment. In re-familiarizing myself with the problem it occurred to me that
I would propose that we add an 'operator->' to ClusterClientCursor::PinnedCursor and remove the 'releaseCursor()' API except within the constructor of a PinnedCursor (via a friend function or some other C++ mechanism). I think that would prevent this type of bug from being reintroduced. Re-flagging this for scheduling since I think it's probably something an intern could pick off and there's probably actually a bug there now. |