[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:
Related
related to SERVER-31120 Invariant failure remotesExhausted_in... Closed
Backwards Compatibility: Fully Compatible
Operating System: ALL
Sprint: Query 2019-07-29, Query 2019-08-12, Query 2019-08-26
Participants:
Case:

 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: SERVER-31138 ensure releaseCursor returns a RAII safe type
Branch: master
https://github.com/mongodb/mongo/commit/a8af39c769926ee06dc6087cbeca8e3da5b943de

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

  1. Line 379 is an early return which looks like it introduced the problem that comment was trying to prevent. Looks like this was the commit which introduced it (cc cheahuychou.mao and misha.tyulenev).
  2. the offending line 371 could just construct the PinnedCursor that we eventually want to return. This wouldn't be easy to do since PinnedCursor doesn't have an operator-> or any way to get at the underlying ClusterClientCursor.
  3. That last sentence is a strange fact. On mongod the ClientCursorPin class has a 'getCursor' and operator->, and the lack of these methods lead to a bunch of boring boilerplate.

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.

Generated at Thu Feb 08 04:26:07 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.