[SERVER-20050] uassert in IDHackStage if on failure to restore the underlying storage cursor (CID 73041) Created: 19/Aug/15  Updated: 16/Aug/23  Resolved: 11/May/23

Status: Closed
Project: Core Server
Component/s: Querying
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Major - P3
Reporter: Coverity Collector User Assignee: Charlie Swanson
Resolution: Duplicate Votes: 0
Labels: coverity, query-44-grooming
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
duplicates SERVER-50838 Coverity analysis defect 115741: Unch... Closed
is duplicated by SERVER-20049 Coverity analysis defect 73042: Unche... Closed
Related
is related to SERVER-23097 Segfault on drop of source collection... Closed
Assigned Teams:
Query Execution
Participants:

 Description   

Value returned from a function is not checked for errors before being used

Defect 73041 (STATIC_C)
Checker CHECKED_RETURN (subcategory none)
File: /src/mongo/db/exec/idhack.cpp
Function mongo::IDHackStage::doRestoreState()
/src/mongo/db/exec/idhack.cpp, line: 209
Calling "restore" without checking return value (as is done elsewhere 7 out of 13 times).

            _recordCursor->restore();



 Comments   
Comment by David Storch [ 02/Aug/19 ]

PlanStage::restoreState() now propagates errors by throwing exceptions. Therefore, I don't think the proposed change of making PlanStage::restoreState() return a Status is relevant anymore. However, we could still consider an improvement in which we make IDHackStage yield recovery check whether the restore() on the underlying cursor was successful, and throw a UserException it if was not. As Rassi mentioned above, though, we don't believe it is possible for the restore() to fail in the context of the IDHackStage. 

Comment by James Wahlin [ 23/Mar/16 ]

Linked to SERVER-23097

When the PlanExecutor in MapReduce is killed while spilling data to disk, we are unable to return an error message with specifics on why we are dead. Returning an error message with PlanExecutor::restoreState() would allow for this.

Comment by J Rassi [ 21/Aug/15 ]

This is not a bug, as RecordCursor::restore() only returns an error when a capped delete overruns the cursor, and the IDHackStage record cursor is unpositioned.

That said, we should change the return type of PlanStage::restoreState() to Status, so that any other errors introduced in the future can be propagated here. This cleanup work will improve the PlanStage interface.

Generated at Thu Feb 08 03:52:59 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.