[WIP] Systematic testing: Test the full KV + History + Consensus stack, with deterministic and randomly-explored interleavings

CMakeLists.txt

@@ -727,6 +727,73 @@ if(BUILD_TESTS)
     )
     target_link_libraries(raft_test PRIVATE ccfcrypto ccf_tasks)
 
+    # Combines a real ccf::kv::Store, a real aft::Aft (raft consensus), and a
+    # real ccf::MerkleTxHistory under real OS-thread concurrency - the three
+    # components production code relies on together, but which no other unit
+    # test suite exercises jointly (kv_test stubs consensus, raft_test stubs
+    # the store, history_test stubs consensus). DETECT_DEADLOCKS is passed
+    # because the checkpoint primitive itself
+    # (src/commit_concurrency/threaded/checkpoint.h) could deadlock if buggy.
+    add_unit_test(
+      commit_concurrency_test
+      ${CMAKE_CURRENT_SOURCE_DIR}/src/commit_concurrency/threaded/main.cpp
+      ${CMAKE_CURRENT_SOURCE_DIR}/src/commit_concurrency/threaded/checkpoint_test.cpp
+      ${CMAKE_CURRENT_SOURCE_DIR}/src/commit_concurrency/threaded/smoke.cpp
+      ${CMAKE_CURRENT_SOURCE_DIR}/src/commit_concurrency/threaded/deterministic.cpp
+      ${CMAKE_CURRENT_SOURCE_DIR}/src/commit_concurrency/threaded/fuzzer.cpp
+      DETECT_DEADLOCKS
+    )
+    set_property(
+      TEST commit_concurrency_test
+      APPEND
+      PROPERTY LABELS concurrency
+    )
+    target_link_libraries(
+      commit_concurrency_test
+      PRIVATE ccfcrypto http_parser ccf_kv ccf_tasks
+    )
+
+    # Systematically explores the space of legal interleavings of a bounded
+    # scenario (rather than sampling timing-dependent ones, as
+    # commit_concurrency_test does), via ccf::kv::test::DeterministicScheduler
+    # in src/commit_concurrency/scheduled/deterministic_scheduler.h -
+    # exhaustively where that space is small enough
+    # (explore_all_interleavings()), or by random sampling where it isn't
+    # (explore_random_interleavings()). DETECT_DEADLOCKS is passed for the
+    # same reason as above.
+    add_unit_test(
+      commit_concurrency_scheduled_test
+      ${CMAKE_CURRENT_SOURCE_DIR}/src/commit_concurrency/scheduled/deterministic_scheduler_test.cpp
+      ${CMAKE_CURRENT_SOURCE_DIR}/src/commit_concurrency/scheduled/main.cpp
+      ${CMAKE_CURRENT_SOURCE_DIR}/src/commit_concurrency/scheduled/pthread_mutex_wrap.cpp
+      ${CMAKE_CURRENT_SOURCE_DIR}/src/commit_concurrency/scheduled/rejected_commit.cpp
+      DETECT_DEADLOCKS
+    )
+    set_property(
+      TEST commit_concurrency_scheduled_test
+      APPEND
+      PROPERTY LABELS concurrency
+    )
+    # ccf_kv/ccf_tasks are linked normally, unmodified, exactly like every
+    # other test target - pthread_mutex_wrap.cpp intercepts their real
+    # ccf::pal::Mutex use at link time instead (see its own comment), so no
+    # source ever needs recompiling against a different Mutex type.
+    target_link_libraries(
+      commit_concurrency_scheduled_test
+      PRIVATE ccfcrypto http_parser ccf_kv ccf_tasks
+    )
+    # See pthread_mutex_wrap.cpp: __wrap_pthread_mutex_lock/unlock/trylock
+    # there are called instead of the real pthread_mutex_lock/unlock/
+    # trylock for every call in this target (__real_pthread_mutex_* is how
+    # they still reach the genuine, original function).
+    target_link_options(
+      commit_concurrency_scheduled_test
+      PRIVATE
+        -Wl,--wrap=pthread_mutex_lock
+        -Wl,--wrap=pthread_mutex_unlock
+        -Wl,--wrap=pthread_mutex_trylock
+    )
+
     add_unit_test(
       raft_enclave_test
       ${CMAKE_CURRENT_SOURCE_DIR}/src/consensus/aft/test/enclave.cpp

include/ccf/ds/locking.h

@@ -7,13 +7,32 @@
 #include <chrono>
 #include <condition_variable>
 #include <mutex>
+#include <source_location>
 #include <utility>
 
 namespace ccf::ds
 {
   class ConditionVariable;
   class MutexGuard;
 
+  namespace detail
+  {
+    // Set immediately before Mutex's own lock()/try_lock()/unlock() make
+    // their real call, and consumed immediately by whatever runs next on
+    // this thread - not read by anything in this header itself. This
+    // lets a genuinely real, immediately-following OS-level lock/unlock
+    // call (which a bare mutex address alone cannot carry a label
+    // through) recover one anyway - see
+    // src/commit_concurrency/scheduled/pthread_mutex_wrap.cpp, which
+    // intercepts real pthread_mutex_lock/unlock/trylock calls to
+    // deterministically explore interleavings, and uses `pending` to
+    // tell a genuine ccf::ds::Mutex call apart from every other,
+    // unrelated lock in the process (allocator, iostream, etc.) without
+    // needing to track any mutex's address at all.
+    inline thread_local bool pending = false;
+    inline thread_local const char* pending_label = nullptr;
+  }
+
   /**
    * Generic locking primitives shared across CCF components.
    */
@@ -31,18 +50,24 @@ namespace ccf::ds
     Mutex(const Mutex&) = delete;
     Mutex& operator=(const Mutex&) = delete;
 
-    void lock() CCF_ACQUIRE()
+    void lock(const char* label = nullptr) CCF_ACQUIRE()
     {
+      detail::pending = true;
+      detail::pending_label = label;
       mutex.lock();
     }
 
-    bool try_lock() CCF_TRY_ACQUIRE(true)
+    bool try_lock(const char* label = nullptr) CCF_TRY_ACQUIRE(true)
     {
+      detail::pending = true;
+      detail::pending_label = label;
       return mutex.try_lock();
     }
 
-    void unlock() CCF_RELEASE()
+    void unlock(const char* label = nullptr) CCF_RELEASE()
     {
+      detail::pending = true;
+      detail::pending_label = label;
       mutex.unlock();
     }
 
@@ -161,4 +186,100 @@ namespace ccf::ds
         lock.get(), timeout_time, std::move(predicate));
     }
   };
+
+  // A drop-in replacement for std::unique_lock<Mutex> (supporting the same
+  // deferred-locking constructor and lock()/try_lock()/unlock() surface
+  // used against ccf::ds::Mutex elsewhere in this codebase), with an
+  // optional label describing why this lock is being taken - passed
+  // directly into Mutex's own lock()/try_lock()/unlock() call. With no
+  // label given, it defaults to the call site's source location.
+  //
+  // Carries its own CCF_SCOPED_CAPABILITY annotations (mirroring
+  // MutexGuard above), rather than relying on Clang's built-in,
+  // name-based special-casing of std::unique_lock, since this needs to
+  // call LockType's own lock()/try_lock()/unlock() directly (to pass a
+  // label through) rather than delegating to a real std::unique_lock
+  // member. This gives real static verification for the ordinary,
+  // unconditional case - a function using this type's lock/unlock like
+  // an ordinary scoped guard is checked exactly as if it used
+  // std::unique_lock or MutexGuard. The one gap: Clang's built-in
+  // std::unique_lock support additionally understands the
+  // conditionally-taken pattern (construct with std::defer_lock, only
+  // sometimes call .lock()/.try_lock() depending on runtime state) well
+  // enough to statically verify it; that specific pattern is not
+  // supported for a user-annotated type like this one, and needs
+  // CCF_NO_THREAD_SAFETY_ANALYSIS on the specific enclosing function that
+  // does it (a handful of call sites in this codebase - see their own
+  // comments for why).
+  template <typename LockType>
+  class CCF_SCOPED_CAPABILITY unique_lock
+  {
+    LockType* mtx;
+    bool owned = false;
+    const char* label;
+    std::source_location loc;
+
+    const char* effective_label() const
+    {
+      return label != nullptr ? label : loc.function_name();
+    }
+
+  public:
+    explicit unique_lock(
+      LockType& mtx_,
+      const char* label_ = nullptr,
+      std::source_location loc_ = std::source_location::current())
+      CCF_ACQUIRE(mtx_) :
+      mtx(&mtx_),
+      label(label_),
+      loc(loc_)
+    {
+      lock();
+    }
+
+    unique_lock(
+      LockType& mtx_,
+      std::defer_lock_t,
+      const char* label_ = nullptr,
+      std::source_location loc_ = std::source_location::current()) :
+      mtx(&mtx_),
+      label(label_),
+      loc(loc_)
+    {}
+
+    ~unique_lock() CCF_RELEASE()
+    {
+      if (owned)
+      {
+        unlock();
+      }
+    }
+
+    void lock() CCF_ACQUIRE()
+    {
+      mtx->lock(effective_label());
+      owned = true;
+    }
+
+    bool try_lock() CCF_TRY_ACQUIRE(true)
+    {
+      const bool locked = mtx->try_lock(effective_label());
+      owned = locked;
+      return locked;
+    }
+
+    void unlock() CCF_RELEASE()
+    {
+      mtx->unlock(effective_label());
+      owned = false;
+    }
+
+    bool owns_lock() const
+    {
+      return owned;
+    }
+
+    unique_lock(const unique_lock&) = delete;
+    unique_lock& operator=(const unique_lock&) = delete;
+  };
 }