[SERVER-30013] opCtx is a naked pointer, except where it's a std::unique_ptr<>. Created: 06/Jul/17  Updated: 23/Jul/19  Resolved: 23/Jul/19

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

Type: Improvement Priority: Major - P3
Reporter: Nathan Myers Assignee: DO NOT USE - Backlog - Platform Team
Resolution: Won't Fix Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Participants:

 Description   

Throughout the server, OperationContexts are operated on via a naked pointer always named opCtx. Often there is a stdx::unique_ptr<OperationContext> (typedef'd "UniqueOperationContext") also named opCtx. There is no way to protect against a null opCtx, or any of myriad implicit, incorrect pointer operations.

  • OperationContext should be renamed OperationContext::Impl.
  • OperationContext itself should be a struct wrapping a std::unique_ptr<OperationContext::Impl>.
  • Uses of OperationContext* in interfaces should be replaced with an OpCtxRef, wrapping a guaranteed non-null OperationContext::Impl*.
  • Member functions of the present OperationContext should be moved to OpCtxRef or to Impl, according to usage.


 Comments   
Comment by Nathan Myers [ 21/Jul/17 ]

> ... a frequent source of programming errors?

It's a legitimate question, but not the one I had chiefly in mind.
A naked pointer is an expensive construct, cognitively,
because in general you don't know which of its properties are
salient to the immediate use. Can it be null? Does arithmetic
on it mean anything? Can its referent expire? Can/must I delete
it? OpCtx is so deeply threaded through the system, we soon
pick up the answers by example, but a newcomer has many other
questions to worry about.

That said, how many use-after-free errors do we have with opCtx?
We have no way of knowing, but we know that it is a common
cause of intermittent failures in similar systems.

The design load might be more important. If it turned out to be
useful to carry state along with each instance, the idea probably
would not reach the conscious level because it is not practical to
do. Given an opaque struct, it would be easy to add a member
alongside the pointer.

Comment by Andy Schwerin [ 06/Jul/17 ]

While this complaint is valid, I'm curious if it's a frequent source of programming errors?

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