Document and test thread safety properties

Closes #2

.clangd

@@ -1,2 +1,2 @@
 CompileFlags:
-  Add: [-DENABLE_MAIN, -UNDEBUG, -DENABLE_FUZZ, -fexceptions]
+  Add: [-DENABLE_MAIN, -UNDEBUG, -DENABLE_FUZZ, -DTHREAD_TEST, -fexceptions]

CMakeLists.txt

@@ -167,6 +167,21 @@ if(BUILD_TESTING)
     add_test(NAME conflict_set_fuzz_${hash} COMMAND fuzz_driver ${TEST})
   endforeach()
 
+  # tsan
+
+  if(NOT CMAKE_CROSSCOMPILING)
+    add_executable(tsan_driver ConflictSet.cpp FuzzTestDriver.cpp)
+    target_compile_options(tsan_driver PRIVATE ${TEST_FLAGS} -fsanitize=thread)
+    target_link_options(tsan_driver PRIVATE -fsanitize=thread)
+    target_compile_definitions(tsan_driver PRIVATE ENABLE_FUZZ THREAD_TEST)
+    target_include_directories(tsan_driver
+                               PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include)
+    foreach(TEST ${CORPUS_TESTS})
+      get_filename_component(hash ${TEST} NAME)
+      add_test(NAME conflict_set_tsan_${hash} COMMAND tsan_driver ${TEST})
+    endforeach()
+  endif()
+
   add_executable(driver TestDriver.cpp)
   target_compile_options(driver PRIVATE ${TEST_FLAGS})
   target_link_libraries(driver PRIVATE ${PROJECT_NAME})

FuzzTestDriver.cpp

@@ -2,15 +2,25 @@
 #include <cstdint>
 #include <fstream>
 #include <sstream>
+#include <thread>
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
 
 int main(int argc, char **argv) {
-  for (int i = 1; i < argc; ++i) {
+  auto doTest = [&]() {
-    std::ifstream t(argv[i], std::ios::binary);
+    for (int i = 1; i < argc; ++i) {
-    std::stringstream buffer;
+      std::ifstream t(argv[i], std::ios::binary);
-    buffer << t.rdbuf();
+      std::stringstream buffer;
-    auto str = buffer.str();
+      buffer << t.rdbuf();
-    LLVMFuzzerTestOneInput((const uint8_t *)str.data(), str.size());
+      auto str = buffer.str();
-  }
+      LLVMFuzzerTestOneInput((const uint8_t *)str.data(), str.size());
+    }
+  };
+#ifdef THREAD_TEST
+  std::thread thread2{doTest};
+#endif
+  doTest();
+#ifdef THREAD_TEST
+  thread2.join();
+#endif
 }

Internal.h

@@ -10,10 +10,12 @@
 #include <cstdlib>
 #include <cstring>
 #include <inttypes.h>
+#include <latch>
 #include <map>
 #include <set>
 #include <span>
 #include <string>
+#include <thread>
 #include <unordered_set>
 #include <utility>
 #include <vector>
@@ -652,32 +654,60 @@ template <class ConflictSetImpl> struct TestDriver {
       auto *results2 =
           new (arena) ConflictSet::Result[numPointReads + numRangeReads];
 
+#ifdef THREAD_TEST
+      auto *results3 =
+          new (arena) ConflictSet::Result[numPointReads + numRangeReads];
+      std::latch ready{1};
+      std::thread thread2{[&]() {
+        ready.count_down();
+        cs.check(reads, results3, numPointReads + numRangeReads);
+      }};
+      ready.wait();
+#endif
+
       CALLGRIND_START_INSTRUMENTATION;
       cs.check(reads, results1, numPointReads + numRangeReads);
       CALLGRIND_STOP_INSTRUMENTATION;
 
       refImpl.check(reads, results2, numPointReads + numRangeReads);
-      for (int i = 0; i < numPointReads + numRangeReads; ++i) {
+
-        if (results1[i] != results2[i]) {
+      auto compareResults = [reads](ConflictSet::Result *results1,
-          if (reads[i].end.len == 0) {
+                                    ConflictSet::Result *results2, int count) {
-            fprintf(stderr,
+        for (int i = 0; i < count; ++i) {
-                    "Expected %s, got %s for read of {%s} at version %" PRId64
+          if (results1[i] != results2[i]) {
-                    "\n",
+            if (reads[i].end.len == 0) {
-                    resultToStr(results2[i]), resultToStr(results1[i]),
+              fprintf(stderr,
-                    printable(reads[i].begin).c_str(), reads[i].readVersion);
+                      "Expected %s, got %s for read of {%s} at version %" PRId64
-          } else {
+                      "\n",
-            fprintf(
+                      resultToStr(results2[i]), resultToStr(results1[i]),
-                stderr,
+                      printable(reads[i].begin).c_str(), reads[i].readVersion);
-                "Expected %s, got %s for read of [%s, %s) at version %" PRId64
+            } else {
-                "\n",
+              fprintf(
-                resultToStr(results2[i]), resultToStr(results1[i]),
+                  stderr,
-                printable(reads[i].begin).c_str(),
+                  "Expected %s, got %s for read of [%s, %s) at version %" PRId64
-                printable(reads[i].end).c_str(), reads[i].readVersion);
+                  "\n",
+                  resultToStr(results2[i]), resultToStr(results1[i]),
+                  printable(reads[i].begin).c_str(),
+                  printable(reads[i].end).c_str(), reads[i].readVersion);
+            }
+            return false;
           }
-          ok = false;
-          return true;
         }
+        return true;
+      };
+
+      if (!compareResults(results1, results2, numPointReads + numRangeReads)) {
+        ok = false;
+        return true;
       }
+
+#ifdef THREAD_TEST
+      thread2.join();
+      if (!compareResults(results3, results2, numPointReads + numRangeReads)) {
+        ok = false;
+        return true;
+      }
+#endif
     }
     return false;
   }

include/ConflictSet.h

@@ -19,7 +19,15 @@ limitations under the License.
 #include <stdint.h>
 
 #ifdef __cplusplus
-
+/** A data structure for optimistic concurrency control on ranges of
+ * bitwise-lexicographically-ordered keys.
+ *
+ * Thread safety:
+ *   - It's safe to operate on two different ConflictSets in two different
+ * threads concurrently
+ *   - It's safe to have multiple threads operating on the same ConflictSet
+ * concurrently if and only if all threads only call `check`.
+ */
 struct __attribute__((__visibility__("default"))) ConflictSet {
   enum Result {
     /** The result of a check which does not intersect any conflicting writes */
@@ -92,6 +100,15 @@ private:
 
 #else
 
+/** A data structure for optimistic concurrency control on ranges of
+ * bitwise-lexicographically-ordered keys.
+ *
+ * Thread safety:
+ *   - It's safe to operate on two different ConflictSets in two different
+ * threads concurrently
+ *   - It's safe to have multiple threads operating on the same ConflictSet
+ * concurrently if and only if all threads only call `check`.
+ */
 typedef struct ConflictSet ConflictSet;
 
 typedef enum {