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

Find a pattern to assert Mutex 'is locked' invariant in user code

    • Type: Icon: Improvement Improvement
    • Resolution: Works as Designed
    • Priority: Icon: Major - P3 Major - P3
    • None
    • Affects Version/s: None
    • Component/s: None
    • Labels:
      None

      The naive implementation is to add methods to the Mutex code directly:

      inline void Mutex::invariantIsLocked() const {    
            invariant(_isLocked,     str::stream() << "Mutex " << getName() << " must be locked");
      }
      inline void Mutex::invariantIsNotLocked() const {    
            invariant(!_isLocked, str::stream() << "Mutex " << getName() << " must be unlocked");
      }
      

      with sample usage pattern to have invariant in a callback that could be passed to the condition_variable:

      auto callback = [this] () -> bool {
         _mutex.invariantIsLocked();
         return _variable > 0;
      }
      
      stdx::unique_lock<Latch> ul(_mutex);
      _conditionVariable.wait(_mutex, callback);

      However it might be unclean to add extra methods at the Mutex layer. billy.donahue said:

      "I would like to get a separate ticket just for this enhancement, and discuss with Service Arch.

      I would like to know more about the WIP use case for this. I'm uneasy with adding a member functions to mongo::Mutex that won't be supported by its base class, and cannot be supported by other mutex implementations like std::mutex. I think the only public API in mongo::Mutex that is non-standard is getName, which really just fills out the mongo::Latch polymorphic interface and is purely for observability in infrastructure code, not really used at user call sites. That is to say, we could change over to std::mutex and only have to change the callers of getName(), which are few and really just internal to stats and diagnostics systems that mutex clients don't normally interact with.

      If anything, I imagine these as nonmember generic functions that TAKE a Lockable or latch or mutex or whatever, and if that Lockable participates in having its locked state observed with isLocked() or something, we can observe it. If a caller wants to invariant on the output of that function, that's their business and not the concern of the platform/mutex.h header. They can just write their own invariants. They'd be better off that way anyway, because they'll have a relevant SourceLocation in the failure, instead of one that just points into the Mutex class."

       

            Assignee:
            billy.donahue@mongodb.com Billy Donahue
            Reporter:
            andrew.shuvalov@mongodb.com Andrew Shuvalov (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: