Added tests for data_race_checker

Author: drowaudio

CMakeLists.txt

@@ -6,4 +6,5 @@ set (CMAKE_CXX_STANDARD 23)
 
 enable_testing()
 add_subdirectory(tests)
-add_subdirectory(include/scl)
+add_subdirectory(include/scl)
+add_subdirectory(include/scl/utils)

README.md

@@ -1,4 +1,5 @@
 # scl - Safe Concurrency Library*
+[![test](https://github.com/drowaudio/scl/actions/workflows/test.yaml/badge.svg)](https://github.com/drowaudio/scl/actions/workflows/test.yaml)
 
 This is an experiment to see how much of sync and send from Rust/Swift can be implemented in C++ and what problems arise when we try to do so. Is there anything we can learn about thread safety from these approaches.
 
@@ -15,6 +16,12 @@ This is an experiment to see how much of sync and send from Rust/Swift can be im
 - Template errors can be difficult to navigate
 - Just having to think about the types passed to scl types (e.g. safe_thread) can help the user think in a more thread-safe way
 
+___
+### Supported Platforms
+- Clang 18
+- GCC 14.2
+- AppleClang 16
+
 ___
 ## To Do:
 ### `sync`/`send`

include/scl/CMakeLists.txt

@@ -7,7 +7,12 @@ include(../../cmake/icm_testing.cmake)
 # Global include for icm based tests
 include_directories("../include")
 
-file(GLOB_RECURSE pass_files LIST_DIRECTORIES false "${CMAKE_CURRENT_SOURCE_DIR}" *.test.cpp)
+icm_glob_build_failure_tests(
+    PATTERN fail_*
+    ERROR_MATCHES ${ERROR_STRING}
+)
+
+file(GLOB pass_files LIST_DIRECTORIES false "${CMAKE_CURRENT_SOURCE_DIR}" pass_*.cpp)
 message(pass_files: ${pass_files})
 
 foreach(pass_file ${pass_files})

include/scl/utils/CMakeLists.txt

@@ -0,0 +1,24 @@
+set(CMAKE_CXX_STANDARD 23)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
+
+include(../../../cmake/icm_build_failure_testing.cmake)
+include(../../../cmake/icm_testing.cmake)
+
+# Global include for icm based tests
+include_directories("../../include")
+
+file(GLOB test_files LIST_DIRECTORIES false "${CMAKE_CURRENT_SOURCE_DIR}" *_test.cpp)
+message(test_file: ${test_files})
+
+foreach(test_file ${test_files})
+    get_filename_component(name_without_extension "${test_file}" NAME_WE)
+    icm_add_test(
+        NAME ${name_without_extension}
+        SOURCES ${test_file}
+    )
+
+    if(MATCHES ${test_file} fail_*)
+        set_tests_properties(${name_without_extension} PROPERTIES
+            WILL_FAIL TRUE)
+    endif()
+endforeach()

include/scl/utils/data_race_checker.h

@@ -4,8 +4,17 @@
 
 #pragma once
 
+#include <chrono>
+#include <cstdint>
+#include <fstream>
+#include <iostream>
+#include <source_location>
+#include <thread>
+#include <vector>
 #include <atomic>
 
+#include "extrinsic_storage.h"
+
 // No Data race:
 // A reader enters a function and there is an active reader
 //
@@ -16,6 +25,12 @@
 //
 // Wait-free? (maybe the CMPX is only lock-free?)
 
+namespace scl {
+
+#ifndef DATA_RACE_DETECTED
+    #define DATA_RACE_DETECTED std::abort();
+#endif
+
 //==========================================
 //==========================================
 struct check_state
@@ -40,19 +55,19 @@ void read_started (check_state& state)
     ++state.num_readers; // must be first
 
     if (state.is_writing)
-        std::terminate();
-        // read during active write
+        DATA_RACE_DETECTED
+    // read during active write
 }
 
 void write_started (check_state& state)
 {
     if (state.is_writing.exchange (true))  // must be first
-        std::terminate();
-        // write during active write
+        DATA_RACE_DETECTED
+    // write during active write
 
     if (state.num_readers > 0)
-        std::terminate();
-        // write during active read
+        DATA_RACE_DETECTED
+    // write during active read
 }
 
 void read_ended (check_state& state)
@@ -65,6 +80,7 @@ void write_ended (check_state& state)
     state.is_writing = false;
 }
 
+#undef DATA_RACE_DETECTED
 
 //==========================================
 //==========================================
@@ -101,24 +117,15 @@ struct scoped_check
 
 //==========================================
 //==========================================
-#include "extrinsic_storage.h"
-
-#include <chrono>
-#include <cstdint>
-#include <fstream>
-#include <iostream>
-#include <source_location>
-#include <thread>
-#include <vector>
-
 template<typename Tag = check_state>
-class date_race_registry {
+class data_race_registry {
     static inline auto tags    = extrinsic_storage<Tag>{};
     static inline auto log     = std::ofstream{ "data-race-violations.log" };
 public:
 
-    static inline auto get_state(void* pobj) noexcept {
-        return *tags.find_or_insert(pobj);
+    static inline auto& get_state(const void* pobj) noexcept {
+        // This const cast should be tidied up
+        return *tags.find_or_insert(const_cast<void*> (pobj));
     }
 
     static inline auto on_destroy(void* pobj) noexcept -> void { tags.erase(pobj); }
@@ -139,17 +146,6 @@ class date_race_registry {
     static inline auto on_write_ended(void* pobj) noexcept -> void {
         if (auto p = tags.find_or_insert(pobj)) { write_ended (*p); }
     }
-
-//    static inline auto on_get_alternative(void* pobj, uint32_t alt, std::source_location where = std::source_location::current()) -> void {
-//        if (auto active = tags.find(pobj);
-//            active                  // if we have discriminator info for this union
-//            && *active != alt       // and the discriminator not what is expected
-//            && *active != unknown   // and is not unknown
-//            )
-//        {
-//            log << where.file_name() << '(' << where.line()
-//                << "): union type safety violation - active member " << (*active == invalid ? "invalid" : std::to_string(*active))
-//                << ", attempted to access " << alt << "\n";
-//        }
-//    }
 };
+
+}

include/scl/utils/data_race_checker.test.h

@@ -0,0 +1,125 @@
+//
+// Created by David Rowland on 11/02/2025.
+//
+
+#include "data_race_checker.h"
+#include <vector>
+#include <thread>
+#include <span>
+#include <atomic>
+
+template <typename T>
+class test_vector
+{
+public:
+  test_vector() {
+    // Create the entry (might allocate)
+    scl::data_race_registry<>::get_state (this);
+  }
+
+  ~test_vector() {
+    scl::data_race_registry<>::on_destroy (this);
+  }
+
+  void reserve (size_t new_cap) {
+    scl::scoped_check<scl::check_type::write> _ (scl::data_race_registry<>::get_state (this));
+    data.resize (new_cap);
+  }
+
+  void clear() {
+    scl::scoped_check<scl::check_type::write> _ (scl::data_race_registry<>::get_state (this));
+    data.clear();
+  }
+
+  void push_back(const T& value) {
+    scl::scoped_check<scl::check_type::write> _ (scl::data_race_registry<>::get_state (this));
+    data.push_back(value);
+  }
+
+  void pop_back() {
+    scl::scoped_check<scl::check_type::write> _ (scl::data_race_registry<>::get_state (this));
+    data.pop_back();
+  }
+
+  T& operator[](size_t index) {
+    scl::scoped_check<scl::check_type::read> _ (scl::data_race_registry<>::get_state (this));
+    return data[index];
+  }
+
+  size_t size() const {
+    scl::scoped_check<scl::check_type::read> _ (scl::data_race_registry<>::get_state (this));
+    return data.size();
+  }
+
+  bool empty() const {
+    scl::scoped_check<scl::check_type::read> _ (scl::data_race_registry<>::get_state (this));
+    return data.empty();
+  }
+
+  size_t capacity() const {
+    scl::scoped_check<scl::check_type::read> _ (scl::data_race_registry<>::get_state (this));
+    return data.capacity();
+  }
+
+private:
+  std::vector<T> data;
+};
+
+inline void test_no_data_race()
+{
+  test_vector<size_t> vec;
+
+  // Fill the vector first
+   for (auto c : std::ranges::iota_view (0uz, 1'000uz))
+     vec.push_back (c);
+
+  // Then only read from the vector - no data race
+std::vector<std::thread> threads;
+
+  for (auto _ : std::ranges::iota_view (0, 3))
+    threads.emplace_back([&]
+                       {
+                         for (auto _ : std::ranges::iota_view (0uz, 1'000'000uz))
+                         {
+                           volatile auto c = 0uz;
+
+                           if (! vec.empty())
+                             c = vec[vec.size() - 1];
+                        }
+                      });
+
+  for (auto& t : threads)
+    t.join();
+}
+
+inline void test_data_race()
+{
+  std::vector<std::thread> threads;
+  test_vector<size_t> vec;
+
+  threads.emplace_back([&]
+                       {
+                         for (;;)
+                         {
+                           for (auto c : std::ranges::iota_view (0uz, 1'000uz))
+                             vec.push_back (c);
+
+                         vec.clear();
+                       }
+                     });
+
+  for (auto _ : std::ranges::iota_view (0, 3))
+    threads.emplace_back([&]
+                       {
+                         for (;;)
+                         {
+                           volatile auto c = 0uz;
+
+                           if (! vec.empty())
+                             c = vec[vec.size() - 1];
+                        }
+                      });
+
+  for (auto& t : threads)
+    t.join();
+}

include/scl/utils/fail_data_race_checker.test.cpp

@@ -0,0 +1,12 @@
+//
+// Created by David Rowland on 11/02/2025.
+//
+
+// Use exit(1) as ctest seems to count a raised signal as a pass...
+#define DATA_RACE_DETECTED std::exit(1);
+#include "data_race_checker.test.h"
+
+int main()
+{
+  test_data_race(); // won't return
+}

include/scl/utils/pass_data_race_checker.test.cpp

@@ -0,0 +1,11 @@
+//
+// Created by David Rowland on 11/02/2025.
+//
+
+#define DATA_RACE_DETECTED std::exit(1);
+#include "data_race_checker.test.h"
+
+int main()
+{
+  test_no_data_race();
+}