Uploaded image for project: 'Core Server'
  1. Core Server
  2. SERVER-89673

Correct lifetime of BSONElement in getTopBottomNValueExprHelper

    • Type: Icon: Bug Bug
    • Resolution: Fixed
    • Priority: Icon: Major - P3 Major - P3
    • 8.1.0-rc0, 8.0.0-rc5
    • Affects Version/s: None
    • Component/s: None
    • Labels:
      None
    • Query Optimization
    • Fully Compatible
    • ALL
    • v8.0
    • 111

      getTopBottomNValueExprHelper returns a BSONElement which may refer to data owned by a BSONObj on the stack.

      The least intrusive change may be to change the helper to return an owning type. All usages immediately call sbe::bson::convertFrom<false /* not a view */>; this could be moved into the helper as this copies out the relevant data.

      BSONObj instances are "optionally" owning of the underlying data. In other cases having BSONElement outlive the BSONObj is safe as the underlying buffer is owned elsewhere (or has shared owners elsewhere), however, this is not guarded in any way; future changes elsewhere can accidentally shorten the lifetime of the true owner, or extend the lifetime of the BSONElement.

      Looking forwards, the lifetime issue around BSONElement could be prevented entirely.

      Possible solutions:

      • Use [[lifetimebound]] annotations to make it a compiler error to have BSONElement outlive BSONObj. This will then always be safe, if the BSONObj itself is safe (the obj owns the buffer, or is correctly outlived by the buffer).
      • Make BSONElement copy the underlying data. Very wasteful in many cases.
      • Make BSONElement a shared owner of the underlying buffer. Not always possible, as BSONObj itself is not always a shared owner, so this would have to devolve to a copy in that case anyway.
      • Split owning vs non-owning in the types, analogous to std::string and std::string_view, with BSONObj, BSONElement, BSONObjView, BSONElementView. This is a "least surprise" option - devs are quite familiar with ...view semantics, and know when and how to use them, and when and how to make a copy instead. Doesn't _prevent misuse, but makes it more obvious. This would be a pretty large refactor, however.

            Assignee:
            james.harrison@mongodb.com James Harrison
            Reporter:
            james.harrison@mongodb.com James Harrison
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved: