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

CMakeLists.txt

@@ -299,6 +299,12 @@ add_ccf_static_library(
     ${CCF_DIR}/src/kv/untyped_map_diff.cpp
   LINK_LIBS ccf_threading
 )
+# Enforced (see kv/store.h, consensus/aft/raft.h, consensus/aft/impl/state.h,
+# node/history.h) so that ccf_kv's compiled objects can safely be linked,
+# unmodified, by every test target - including one that recompiles those
+# headers with a different ccf::pal::Mutex (see
+# src/commit_concurrency/interleaving_lock_override.h) - without an ODR violation.
+target_compile_definitions(ccf_kv PRIVATE CCF_STATIC_LIBRARY_BUILD)
 
 # CCF endpoints lib
 add_ccf_static_library(
@@ -338,6 +344,8 @@ add_ccf_static_library(
     ${CCF_DIR}/src/tasks/worker.cpp
   LINK_LIBS ccf_threading
 )
+# See the comment on ccf_kv's own CCF_STATIC_LIBRARY_BUILD above.
+target_compile_definitions(ccf_tasks PRIVATE CCF_STATIC_LIBRARY_BUILD)
 
 find_library(BACKTRACE_LIBRARY backtrace)
 if(NOT BACKTRACE_LIBRARY)
@@ -720,6 +728,91 @@ 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 interleaving primitive itself (src/commit_concurrency/interleaving.h)
+    # could deadlock if buggy.
+    add_unit_test(
+      commit_concurrency_test
+      ${CMAKE_CURRENT_SOURCE_DIR}/src/commit_concurrency/interleaving_test.cpp
+      ${CMAKE_CURRENT_SOURCE_DIR}/src/commit_concurrency/threaded/main.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
+    )
+
+    # Explores every legal interleaving of a bounded scenario (rather than
+    # sampling timing-dependent ones, as commit_concurrency_test does)
+    # via ccf::kv::test::explore_all_interleavings() in
+    # src/commit_concurrency/deterministic_scheduler.h. DETECT_DEADLOCKS is passed for
+    # the same reason as above.
+    add_unit_test(
+      commit_concurrency_model_test
+      ${CMAKE_CURRENT_SOURCE_DIR}/src/commit_concurrency/deterministic_scheduler_test.cpp
+      ${CMAKE_CURRENT_SOURCE_DIR}/src/commit_concurrency/model_checked/main.cpp
+      ${CMAKE_CURRENT_SOURCE_DIR}/src/commit_concurrency/model_checked/rejected_commit_stall.cpp
+      # ccf::tasks' own sources (normally built once into ccf_tasks and
+      # shared unmodified - see kv_test's use of ccf_kv, for example) are
+      # rebuilt here instead of linking ccf_tasks, so that they see the
+      # same -include below as everything else in this target: ccf::tasks
+      # keeps a process-wide job board (a real ccf::pal::Mutex user) that
+      # outlives any single explored schedule, so every thread that can
+      # reach it - including any of ccf::tasks' own internals - needs the
+      # same scheduler-aware lock for DriverRegistration (see
+      # deterministic_scheduler.h) to keep it consistent across schedules.
+      ${CCF_DIR}/src/tasks/task_system.cpp
+      ${CCF_DIR}/src/tasks/job_board.cpp
+      ${CCF_DIR}/src/tasks/ordered_tasks.cpp
+      ${CCF_DIR}/src/tasks/fan_in_tasks.cpp
+      ${CCF_DIR}/src/tasks/thread_manager.cpp
+      ${CCF_DIR}/src/tasks/worker.cpp
+      DETECT_DEADLOCKS
+    )
+    set_property(
+      TEST commit_concurrency_model_test
+      APPEND
+      PROPERTY LABELS concurrency
+    )
+    # The -include flag makes every source file in this target (and only
+    # this target) see ccf::pal::Mutex itself resolve to SchedulerMutex -
+    # see src/commit_concurrency/interleaving_lock_override.h.
+    target_compile_options(
+      commit_concurrency_model_test
+      PRIVATE
+        -include
+        ${CMAKE_CURRENT_SOURCE_DIR}/src/commit_concurrency/interleaving_lock_override.h
+    )
+    # ccf_kv and ccfcrypto are safe to share, unmodified, with every other
+    # test target here despite the -include above: none of their own
+    # sources include store.h, raft.h, impl/state.h, or history.h, and
+    # each of those four headers refuses to compile at all into either of
+    # them (CCF_STATIC_LIBRARY_BUILD, set on both below), so this stops
+    # being true loudly, at build time, rather than silently. ccf_tasks is
+    # deliberately not linked here - see the comment on its sources above.
+    target_link_libraries(
+      commit_concurrency_model_test
+      PRIVATE
+        ccfcrypto
+        http_parser
+        ccf_kv
+        ccf_threading
+        ${CMAKE_DL_LIBS}
+        ${BACKTRACE_LIBRARY}
+    )
+
     add_unit_test(
       raft_enclave_test
       ${CMAKE_CURRENT_SOURCE_DIR}/src/consensus/aft/test/enclave.cpp

cmake/crypto.cmake

@@ -35,6 +35,8 @@ find_library(TLS_LIBRARY ssl)
 
 add_library(ccfcrypto STATIC ${CCFCRYPTO_SRC})
 add_warning_checks(ccfcrypto)
+# See the comment on ccf_kv's own CCF_STATIC_LIBRARY_BUILD in CMakeLists.txt.
+target_compile_definitions(ccfcrypto PRIVATE CCF_STATIC_LIBRARY_BUILD)
 target_compile_options(
   ccfcrypto
   PRIVATE $<$<COMPILE_LANG_AND_ID:CXX,Clang>:-Wno-vla-cxx-extension>

include/ccf/pal/locking.h

@@ -6,13 +6,28 @@
 
 #include <condition_variable>
 #include <mutex>
+#include <source_location>
 #include <utility>
 
 namespace ccf::pal
 {
   class ConditionVariable;
   class MutexGuard;
 
+#if defined(CCF_TEST_INTERLEAVING_LOCK_TYPE)
+  // A test build may define this (before this header is first included,
+  // via a -include compiler flag applying to every source file in that
+  // build) to replace ccf::pal::Mutex itself, everywhere, with a different,
+  // instrumented lock type - see that type's own declaration for what it
+  // does instead of real locking. MutexGuard and ConditionVariable below
+  // are both written against the name Mutex, so they bind to whichever
+  // type this resolves to; the replacement type must therefore expose the
+  // same public lock()/try_lock()/unlock() surface, and (for
+  // ConditionVariable::wait() and friends to keep compiling) a private
+  // member also named `mutex`, friended to ConditionVariable, of type
+  // std::mutex.
+  using Mutex = CCF_TEST_INTERLEAVING_LOCK_TYPE;
+#else
   /**
    * Virtual enclaves and the host code share the same PAL.
    */
@@ -50,6 +65,7 @@ namespace ccf::pal
       return mutex.native_handle();
     }
   };
+#endif
 
   class CCF_SCOPED_CAPABILITY MutexGuard
   {
@@ -160,4 +176,92 @@ namespace ccf::pal
         lock.get(), timeout_time, std::move(predicate));
     }
   };
+
+  // Called (if non-null) whenever a ccf::pal::unique_lock below actually
+  // acquires its lock, with a short label describing why - either given
+  // explicitly at the call site, or (if not) a source-location-derived
+  // default. Null outside of test code that wants to observe this; see
+  // src/commit_concurrency/deterministic_scheduler.h's SchedulerThreadContext,
+  // the one place that currently sets it, forwarding to
+  // DeterministicScheduler::set_action() so a failing scenario's
+  // describe() can show real semantic reasons at real lock points, not
+  // just its own explicit yield_point() labels. Deliberately not
+  // thread_local: the one place that installs it already reads its own
+  // thread-local state to decide whether the calling thread has an active
+  // scheduler, so this only ever needs a single, one-time global install.
+  using LockLabelSink = void (*)(const char* label);
+  inline LockLabelSink lock_label_sink = nullptr;
+
+  // A drop-in replacement for std::unique_lock<Mutex> (supporting the same
+  // deferred-locking constructor and lock()/try_lock()/unlock() surface
+  // used against ccf::pal::Mutex elsewhere in this codebase), with an
+  // optional label describing why this lock is being taken - reported to
+  // lock_label_sink above every time this actually acquires the lock. With
+  // no label given, the label defaults to the call site's source location.
+  template <typename LockType>
+  class unique_lock
+  {
+    std::unique_lock<LockType> inner;
+    const char* label;
+    std::source_location loc;
+
+    void report_if_locked()
+    {
+      if (inner.owns_lock() && lock_label_sink != nullptr)
+      {
+        lock_label_sink(label != nullptr ? label : loc.function_name());
+      }
+    }
+
+  public:
+    explicit unique_lock(
+      LockType& mtx,
+      const char* label_ = nullptr,
+      std::source_location loc_ = std::source_location::current()) :
+      inner(mtx),
+      label(label_),
+      loc(loc_)
+    {
+      report_if_locked();
+    }
+
+    unique_lock(
+      LockType& mtx,
+      std::defer_lock_t defer,
+      const char* label_ = nullptr,
+      std::source_location loc_ = std::source_location::current()) :
+      inner(mtx, defer),
+      label(label_),
+      loc(loc_)
+    {}
+
+    void lock()
+    {
+      inner.lock();
+      report_if_locked();
+    }
+
+    bool try_lock()
+    {
+      const bool locked = inner.try_lock();
+      if (locked)
+      {
+        report_if_locked();
+      }
+      return locked;
+    }
+
+    void unlock()
+    {
+      inner.unlock();
+    }
+
+    bool owns_lock() const
+    {
+      return inner.owns_lock();
+    }
+
+    unique_lock(const unique_lock&) = delete;
+    unique_lock& operator=(const unique_lock&) = delete;
+  };
 }