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

ExpressionDateFromString::evaluate() is not thread safe

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical - P2
    • Resolution: Fixed
    • Affects Version/s: 3.6.0-rc4
    • Fix Version/s: 3.6.0-rc5, 3.7.1
    • Component/s: Aggregation Framework
    • Labels:
      None
    • Backwards Compatibility:
      Fully Compatible
    • Operating System:
      ALL
    • Backport Requested:
      v3.6
    • Sprint:
      Query 2017-12-04
    • Linked BF Score:
      0

      Description

      In general, aggregation expressions are owned by a single thread responsible for executing the aggregate command. However, this is not the case for a document validator which uses $expr. A thread running a create or collMod commands sets up the validator. Subsequent writes to the collection can concurrently evaluate the aggregation expression, as part of $expr. Note that this is the only the case for storage engines which support document-level concurrency; on MMAPv1, writes will take an exclusive collection lock and thus will not evaluate the $expr concurrently.

      Expression::evaluate() is marked as const and therefore is expected to be thread-safe, but ExpressionDateFromString's evaluate implementation has a subtle data race. This expression holds a subexpression by boost::intrusive_ptr:

      https://github.com/mongodb/mongo/blob/3b5249593ad3bdbc8799f6a2f80ba89568f7894a/src/mongo/db/pipeline/expression.h#L879

      It passes this intrusive_ptr by value to the makeTimeZone() helper:

      https://github.com/mongodb/mongo/blob/3b5249593ad3bdbc8799f6a2f80ba89568f7894a/src/mongo/db/pipeline/expression.cpp#L1376

      This causes the intrusive_ptr refcount to be incremented when this function is called, and decremented when the function returns. This increment/decrement pair amounts to mutating the object inside a const method, and it results in a data race. The Expression class inherits from IntrusiveCounterUnsigned in order to interface with instrusive_ptr. IntrusiveCounterUnsigned writes directly to an unsigned integer on increment and decrement:

      https://github.com/mongodb/mongo/blob/3b5249593ad3bdbc8799f6a2f80ba89568f7894a/src/mongo/util/intrusive_counter.h#L86

      This is in contrast to our RefCountable class, whose refcount is an AtomicUInt32.

        Attachments

          Activity

            People

            Assignee:
            david.storch David Storch
            Reporter:
            david.storch David Storch
            Participants:
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: