diff --git a/.gitea/workflows/ci.yaml b/.gitea/workflows/ci.yaml index 7113c07..d46f8f8 100644 --- a/.gitea/workflows/ci.yaml +++ b/.gitea/workflows/ci.yaml @@ -67,6 +67,34 @@ jobs: fail_on_failure: 'false' - run: echo "🍏 This job's status is ${{ job.status }}." + thread_sanitizer: + runs-on: ubuntu-latest + steps: + - name: Check out repository code + uses: actions/checkout@v4 + - name: dependencies + run: | + sudo apt-get update -y + sudo apt-get install -y cmake gcc moreutils + # The thread tests assert exclusive ownership of pool slots and reserved + # ranges, which is checkable without tooling and runs in the job above. + # This is the run that proves there is no data race underneath them. + # libtsan arrives with gcc (libgcc-N-dev depends on it); the script + # disables ASLR because TSan aborts on kernels with vm.mmap_rnd_bits > 28. + - name: thread sanitizer + run: | + scripts/thread_test.sh build/tsan --output-junit "$(pwd)/tsan-junit.xml" + - name: publish thread sanitizer results + if: always() + uses: mikepenz/action-junit-report@v4 + with: + report_paths: 'tsan-junit.xml' + annotate_only: true + detailed_summary: true + include_passed: true + fail_on_failure: 'true' + - run: echo "🍏 This job's status is ${{ job.status }}." + mutation_test: runs-on: ubuntu-latest steps: diff --git a/AGENTS.md b/AGENTS.md index 9db9382..735a651 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -3,11 +3,13 @@ ## Project Structure & Module Organization This is a small C library built with CMake. Core implementation lives in -`src/error.c`. The public header is generated at build time from -`include/akerror.tmpl.h` by `scripts/generrno.sh`, which also generates -`src/errno.c` under the build directory. CMake package and pkg-config templates -are in `cmake/` and `akerror.pc.in`. Tests are one-file C programs in `tests/`; -shared test helpers live beside them, such as `tests/err_capture.h`. +`src/error.c`. `src/lock.h` is private: it selects the threading backend +(pthread or none) and is not installed. The public header is generated at build +time from `include/akerror.tmpl.h` by `scripts/generrno.sh`, which also +generates `src/errno.c` under the build directory and stamps in whether the +build is thread safe. CMake package and pkg-config templates are in `cmake/` and +`akerror.pc.in`. Tests are one-file C programs in `tests/`; shared test helpers +live beside them, such as `tests/err_capture.h` and `tests/err_threads.h`. ## Build, Test, and Development Commands @@ -27,6 +29,20 @@ runs the registered unit tests. To emit CI-style results, use: ctest --test-dir build --output-on-failure --output-junit "$(pwd)/ctest-junit.xml" ``` +The suite includes thread tests. The run that proves there is no data race under +them is ThreadSanitizer: + +```sh +scripts/thread_test.sh +``` + +which configures `build/tsan` with `-DAKERR_SANITIZE=thread`, builds the library +*and* the tests with it, and runs CTest with ASLR disabled (TSan aborts with an +"unexpected memory mapping" on kernels with `vm.mmap_rnd_bits` above 28). +`AKERR_SANITIZE` takes any sanitizer list, so `-DAKERR_SANITIZE=address,undefined` +works the same way. Under a sanitized build a report fails the test rather than +being printed and passed over. + Mutation testing is available through: ```sh @@ -44,9 +60,13 @@ scripts/coverage.py --threshold 90 --branch-threshold 50 `scripts/coverage.py` configures its own instrumented build tree (default `build/coverage`, `-DAKERR_COVERAGE=ON`), runs the CTest suite there, and reports gcov line/branch coverage per library source. Thresholds gate each -file as well as the total. Coverage measures `src/error.c` and the generated -`src/errno.c` only; the public header's macros expand at their call sites, so -mutation testing is what checks those. +file as well as the total. Coverage measures the library's own sources only -- +`src/error.c`, `src/lock.h` and the generated `src/errno.c`; the public header's +macros expand at their call sites, so mutation testing is what checks those. + +To build without threads (no locking, no thread-local storage, thread tests not +registered), configure with `-DAKERR_THREADS=none`. `auto` is the default and +fails the configure rather than falling back. ## Coding Style & Naming Conventions @@ -56,6 +76,15 @@ such as `PREPARE_ERROR`. Keep generated-code changes in templates or generator scripts, not in build outputs. Preserve concise comments for invariants, macro constraints, and non-obvious error lifecycle behavior. +**Locking.** One recursive lock (`akerr_state_lock`, see `src/lock.h`) covers +both the error pool and the status registry. A function named `*_locked` is +called with that lock already held; a function without the suffix takes it. +Never take it in a body written with the `FAIL_*_RETURN` macros — those return +from the middle of the function and would skip the unlock. Split it instead: the +locked body does the work, and a thin wrapper takes the lock, calls it, and +releases it on the single return path. Do not call consumer code +(`akerr_log_method`, `akerr_handler_unhandled_error`) while holding it. + ## Testing Guidelines Add tests as `tests/err_.c`. Register each new test in the @@ -66,6 +95,19 @@ CTest suite before submitting changes, and run mutation testing and coverage when changing core control-flow, reference counting, stack-trace, or handler behavior. +Tests that drive the library from several threads go in the `AKERR_THREAD_SAFE` +branch of the `AKERR_TESTS` list — an `-DAKERR_THREADS=none` build has no +threading to test — and use `tests/err_threads.h`, which runs a body on +`AKERR_TEST_THREADS` threads that meet at a barrier first. +Count failures per thread with `AKERR_TCHECK` rather than returning early: a +thread that abandons its work leaves the others holding pool slots and turns one +failure into a cascade. Anything the test shares between its own threads must go +through `__atomic` builtins — a race in the test is still a race, and +ThreadSanitizer cannot tell you whose it is. The capturing logger in +`err_capture.h` is single-threaded (shared buffer, shared length); use +`akerr_thread_logger` instead. Run `scripts/thread_test.sh` when changing +anything that touches the pool, the registry, initialization, or the lock. + ## Commit & Pull Request Guidelines Recent commits use short, imperative, sentence-case subjects, for example diff --git a/CMakeLists.txt b/CMakeLists.txt index c4368cc..6fafa76 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,9 +1,12 @@ cmake_minimum_required(VERSION 3.10) -# The status-code registry replaced the consumer-sized __AKERR_ERROR_NAMES array -# with private storage, which is a source and ABI break for anything built -# against an earlier header -- hence 1.0.0 and a SOVERSION, so a stale installed -# libakerror.so can no longer be silently paired with new headers. -project(akerror VERSION 1.0.0 LANGUAGES C) +# 1.0.0 replaced the consumer-sized __AKERR_ERROR_NAMES array with private +# storage. 2.0.0 makes the library thread safe, which is a second ABI break in +# the same places: __akerr_last_ignored became thread-local storage, and +# ENSURE_ERROR_READY no longer takes the pool reference that akerr_next_error() +# now takes for it. Consumer code compiled against a 1.x header would +# double-count every reference. Hence the major bump and the SOVERSION, so a +# stale installed libakerror.so cannot be silently paired with new headers. +project(akerror VERSION 2.0.0 LANGUAGES C) include(GNUInstallDirs) include(CMakePackageConfigHelpers) @@ -11,6 +14,39 @@ include(CTest) set(AKERR_USE_STDLIB 1 CACHE BOOL "Use the C standard library") set(AKERR_COVERAGE 0 CACHE BOOL "Instrument the build with gcov coverage counters") +set(AKERR_SANITIZE "" CACHE STRING + "Sanitizers to build the library and tests with, e.g. thread or address,undefined") + +# Threading backend for the library's global state (the error pool and the +# status registry). "auto" takes POSIX threads when they exist and fails the +# configure when they do not: a build that silently fell back to no locking +# would produce a library that reports itself thread safe and is not. Say +# -DAKERR_THREADS=none to mean it on purpose. +set(AKERR_THREADS "auto" CACHE STRING "Threading backend: auto, pthread, or none") +set_property(CACHE AKERR_THREADS PROPERTY STRINGS auto pthread none) + +if(AKERR_THREADS STREQUAL "auto" OR AKERR_THREADS STREQUAL "pthread") + set(THREADS_PREFER_PTHREAD_FLAG ON) + find_package(Threads) + if(CMAKE_USE_PTHREADS_INIT) + set(AKERR_THREAD_SAFE 1) + elseif(AKERR_THREADS STREQUAL "pthread") + message(FATAL_ERROR + "-DAKERR_THREADS=pthread was requested but no POSIX thread library " + "was found.") + else() + message(FATAL_ERROR + "No POSIX thread library was found. libakerror serializes its " + "global state with a recursive pthread mutex; without one it " + "cannot be thread safe. Configure with -DAKERR_THREADS=none to " + "build a deliberately single-threaded library instead.") + endif() +elseif(AKERR_THREADS STREQUAL "none") + set(AKERR_THREAD_SAFE 0) +else() + message(FATAL_ERROR + "AKERR_THREADS must be auto, pthread or none, not '${AKERR_THREADS}'") +endif() # Size of the private status-name hash table. Must be a power of two; usable # capacity is 75% of it (src/error.c asserts both). The host's errno list @@ -59,6 +95,25 @@ function(akerr_instrument_for_coverage _target) endif() endfunction() +# Sanitizers. Unlike coverage these go on the tests as well as the library: +# ThreadSanitizer only sees a race if every thread that touches the memory was +# compiled with it, and the threads live in the test programs. +# cmake -S . -B build/tsan -DAKERR_SANITIZE=thread +if(AKERR_SANITIZE AND NOT CMAKE_C_COMPILER_ID MATCHES "GNU|Clang") + message(FATAL_ERROR + "AKERR_SANITIZE requires GCC or Clang, not ${CMAKE_C_COMPILER_ID}") +endif() + +function(akerr_instrument_for_sanitizers _target) + if(AKERR_SANITIZE) + target_compile_options(${_target} PRIVATE + -fsanitize=${AKERR_SANITIZE} + -fno-omit-frame-pointer -g -O1) + set_property(TARGET ${_target} APPEND_STRING + PROPERTY LINK_FLAGS " -fsanitize=${AKERR_SANITIZE}") + endif() +endfunction() + set(SCRIPT ${CMAKE_CURRENT_SOURCE_DIR}/scripts/generrno.sh) set(INFILE ${CMAKE_CURRENT_SOURCE_DIR}/include/akerror.tmpl.h) @@ -67,6 +122,14 @@ set(GENERATED_DIR ${CMAKE_CURRENT_BINARY_DIR}/generated) set(GENERATED_ERRNO_C ${GENERATED_DIR}/src/errno.c) set(GENERATED_AKERROR_H ${GENERATED_DIR}/include/akerror.h) +# The threading decision is stamped into the generated header, so the header has +# to be regenerated when it changes. Makefile generators compare timestamps +# rather than command lines, so carry the value through a file: configure_file +# rewrites it only when the content differs, which is exactly the trigger we +# want and no trigger at all on an unchanged reconfigure. +set(GENERATED_THREAD_STAMP ${CMAKE_CURRENT_BINARY_DIR}/akerr_thread_safe.stamp) +configure_file(cmake/thread_safe.stamp.in ${GENERATED_THREAD_STAMP} @ONLY) + add_custom_command( OUTPUT ${GENERATED_ERRNO_C} ${GENERATED_AKERROR_H} COMMAND ${CMAKE_COMMAND} -E make_directory ${GENERATED_DIR} @@ -74,7 +137,8 @@ add_custom_command( ${SCRIPT} ${CMAKE_CURRENT_SOURCE_DIR} ${GENERATED_DIR} - DEPENDS ${SCRIPT} ${INFILE} + ${AKERR_THREAD_SAFE} + DEPENDS ${SCRIPT} ${INFILE} ${GENERATED_THREAD_STAMP} VERBATIM ) @@ -91,18 +155,34 @@ target_include_directories(akerror PUBLIC find_package(PkgConfig REQUIRED) add_library(akerror::akerror ALIAS akerror) +# The threading backend is PRIVATE: src/lock.h is not installed, so which +# primitive the library locks with is invisible to a consumer. What a consumer +# does see -- whether the library locks at all -- travels in the generated +# header instead, where it cannot disagree with this build. +if(AKERR_THREAD_SAFE) + set(AKERR_THREADS_DEFINE AKERR_THREADS_PTHREAD=1) +else() + set(AKERR_THREADS_DEFINE AKERR_THREADS_NONE=1) +endif() + target_compile_definitions(akerror PUBLIC AKERR_USE_STDLIB=${AKERR_USE_STDLIB} PRIVATE AKERR_STATUS_NAME_SLOTS=${AKERR_STATUS_NAME_SLOTS} PRIVATE AKERR_MAX_RESERVED_STATUS_RANGES=${AKERR_MAX_RESERVED_STATUS_RANGES} + PRIVATE ${AKERR_THREADS_DEFINE} ) +if(AKERR_THREAD_SAFE) + target_link_libraries(akerror PRIVATE Threads::Threads) +endif() + set_target_properties(akerror PROPERTIES VERSION ${PROJECT_VERSION} SOVERSION ${PROJECT_VERSION_MAJOR} ) akerr_instrument_for_coverage(akerror) +akerr_instrument_for_sanitizers(akerror) # Each test is one source file in tests/ built into test_ and registered # as CTest . Tests expected to abort (unhandled error / contract @@ -141,6 +221,18 @@ set(AKERR_TESTS err_release_refcount ) +# These drive the library from many threads at once. They are worth running on +# their own -- they assert exclusive ownership of pool slots and of reserved +# ranges, which is checkable without a sanitizer -- but the run that proves the +# absence of a race is the one under -DAKERR_SANITIZE=thread. +if(AKERR_THREAD_SAFE) + list(APPEND AKERR_TESTS + err_threads_init + err_threads_pool + err_threads_registry + ) +endif() + set(AKERR_WILL_FAIL_TESTS err_trace err_improper_closure @@ -151,7 +243,19 @@ foreach(_test IN LISTS AKERR_TESTS) add_executable(test_${_test} tests/${_test}.c) target_include_directories(test_${_test} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/tests) target_link_libraries(test_${_test} PRIVATE akerror) + if(AKERR_THREAD_SAFE) + target_link_libraries(test_${_test} PRIVATE Threads::Threads) + endif() + akerr_instrument_for_sanitizers(test_${_test}) add_test(NAME ${_test} COMMAND test_${_test}) + # A sanitizer report is a test failure. Without halt_on_error the runtime + # prints and continues, which leaves a race to be noticed in the log by + # somebody reading it -- and under a race storm the reporting itself is slow + # enough to look like a hang. + if(AKERR_SANITIZE) + set_tests_properties(${_test} PROPERTIES ENVIRONMENT + "TSAN_OPTIONS=halt_on_error=1;ASAN_OPTIONS=halt_on_error=1;UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1") + endif() endforeach() set_tests_properties( diff --git a/README.md b/README.md index 2aed77a..ca490ce 100644 --- a/README.md +++ b/README.md @@ -4,12 +4,14 @@ This library provides a TRY/CATCH style exception handling mechanism for C. ![build badge](https://source.starfort.tech/andrew/libakerror/actions/workflows/ci.yaml/badge.svg?branch=main) -## Upgrading from a pre-1.0.0 release +## Upgrading -1.0.0 replaced the consumer-sized status-name array with a private, ownership- -enforced registry. That is a source and ABI break: see -[UPGRADING.md](UPGRADING.md) for what was removed, how to migrate, the capacity -limits and how to raise them, and the thread-safety rules. +2.0.0 makes the library thread safe. That is an ABI break — `__akerr_last_ignored` +became thread-local storage and the pool now takes its own reference — so +everything built against a 1.x header must be rebuilt. 1.0.0 replaced the +consumer-sized status-name array with a private, ownership-enforced registry. +See [UPGRADING.md](UPGRADING.md) for both, what was removed, how to migrate, the +capacity limits and how to raise them, and the thread-safety rules. # Why? @@ -114,6 +116,90 @@ functions raise, the capacity limits and how to raise them, and thread-safety rules. +# Thread safety + +The library is thread safe as built by default. Every entry point may be called +from any thread at any time, including the first one: `akerr_init()` runs +exactly once no matter how many threads race into it. + +What that covers: + +* **The error pool.** Finding a free slot in `AKERR_ARRAY_ERROR` and taking its + reference is one operation under a lock, so two threads can never be handed + the same context. A context is then owned by the thread that raised it, all + the way through `CATCH`, `HANDLE`, and release. +* **The status registry.** Reservations and name registrations are serialized + against each other and against lookups. Two threads reserving the same range + cannot both win — exactly one gets `NULL` and the other gets + `AKERR_STATUS_RANGE_OVERLAP` naming the winner. +* **Per-thread state.** The context behind `IGNORE` (`__akerr_last_ignored`) and + the last-ditch context used to report `akerr_release_error(NULL)` are + thread-local, so one thread's ignored error is never another's. + +What it does not cover, and cannot: + +* **Sharing one error context between threads.** The library hands a context to + one thread; passing it to another is your synchronization to do. +* **`akerr_log_method` and `akerr_handler_unhandled_error`.** Set them during + startup, before you spawn threads. They are read on every error and the + library never writes them after initialization, so setting one while other + threads are raising errors is a race the library cannot mediate. +* **Renaming a status that other threads are looking up.** + `akerr_name_for_status(status, NULL)` returns a pointer into the registry, + valid for the life of the process; registering a *second* name for the same + status overwrites that buffer in place. Register names during initialization. + Registering a *new* status concurrently is fine. +* **Which unhandled error terminates the process.** An error that reaches + `FINISH_NORETURN` unhandled prints its stack trace and calls + `akerr_handler_unhandled_error`, which by default calls `exit()`. Each + thread's trace is whole — the buffer belongs to its context, and each line is + one call to `akerr_log_method` — but if two threads get there at the same + instant, both traces print and the exit status is whichever one won. + +There is one lock, it is recursive, and it covers both the pool and the +registry. That means error construction is serialized across threads: raising an +error is the exceptional path, and correctness there is worth more than +throughput. A program that raises errors on its hot path will feel it. + +`AKERR_THREAD_SAFE` in the generated header is `1` for a thread-safe build, so a +consumer can check what it linked against: + +```c +#if AKERR_THREAD_SAFE + /* ... start worker threads ... */ +#endif +``` + +## Building single threaded + +The threading backend is chosen when libakerror is configured. `auto` (the +default) takes POSIX threads, and **fails the configure** if it cannot find +them rather than quietly producing a library that says it is thread safe and is +not. To mean it: + +```sh +cmake -S . -B build -DAKERR_THREADS=none +``` + +That builds with no locking and no thread-local storage, stamps +`AKERR_THREAD_SAFE 0` into the header, and calling the library from more than +one thread is then undefined. + +## Proving it + +The thread tests (`tests/err_threads_*.c`) assert the properties above directly: +exclusive ownership of pool slots, exactly one winner for a contested range, +every registered name readable back. They run in the normal suite. The run that +proves the *absence* of a data race underneath them is ThreadSanitizer: + +```sh +scripts/thread_test.sh +``` + +which configures `build/tsan` with `-DAKERR_SANITIZE=thread`, builds the library +and every test with it, and runs the suite. Under that build a sanitizer report +fails the test rather than being printed and passed over. + # Installation ```bash @@ -132,7 +218,8 @@ The build process relies upon `scripts/generrno.sh` which performs the following ## Dependencies -This library depends upon `stdlib`. If you don't want to link against stdlib, you must modify the library code to include headers and link against a library that provides the following: +This library depends upon `stdlib`, and upon POSIX threads unless it is built +with `-DAKERR_THREADS=none` (see "Thread safety" above). If you don't want to link against stdlib, you must modify the library code to include headers and link against a library that provides the following: - `memset` function - `strncpy` function diff --git a/TODO.md b/TODO.md index ba66fa0..19fb819 100644 --- a/TODO.md +++ b/TODO.md @@ -2,18 +2,20 @@ Working notes for `libakerror`. Outstanding items only. -## 1. The test suite has no sanitizer run +## 1. Only ThreadSanitizer is wired into CI, not ASan/UBSan -Mutation testing caught an out-of-bounds probe in the status-name hash table -that the suite could not: the failure mode was a write into adjacent BSS, which -does not crash, so every test still passed. Sharpening one test closed that -instance, but ASan would have caught the whole class directly and independently -of how sharp the assertions are. +`AKERR_SANITIZE` builds the library and the tests with any sanitizer list, and +CI runs `-DAKERR_SANITIZE=thread` through `scripts/thread_test.sh`. Nothing runs +`address,undefined` yet, and that is the one that covers the original +motivation: mutation testing caught an out-of-bounds probe in the status-name +hash table that the suite could not, because the failure mode was a write into +adjacent BSS, which does not crash. Sharpening one test closed that instance; +ASan would catch the whole class regardless of how sharp the assertions are. -Add a `-fsanitize=address,undefined` build to `.gitea/workflows/ci.yaml`, or a -CMake option alongside `AKERR_COVERAGE`. This is the highest-value item here: it -covers the whole library, not just the registry, and the library's fixed pools -and manual buffer arithmetic are exactly what it is good at. +The machinery is in place — this is one more job in +`.gitea/workflows/ci.yaml` running +`cmake -S . -B build/asan -DAKERR_SANITIZE=address,undefined`. Left separate +because ASan and TSan cannot be combined in one build. ## 2. `HANDLE`-level status aliasing is still undetectable @@ -48,12 +50,23 @@ A plugin host that `dlopen`s many distinct plugins over a process lifetime accumulates ranges until the table fills. Reloading the *same* plugin is fine — an identical repeat by the same owner is idempotent. -## 5. The registry is not thread safe +## 5. Renaming a status is not safe against a concurrent lookup -Global mutable state, no locking, and `akerr_status_name_count++` is not atomic. -Currently documented as an initialization-time-only API rather than enforced. If -components start initializing on separate threads this needs either a lock or a -documented once-per-process init barrier. +`akerr_name_for_status(status, NULL)` returns a pointer into the registry rather +than a copy, which is what makes it usable from inside `FAIL` — it needs no +buffer and no error context of its own. Registering a *second* name for a status +that already has one (`tests/err_name_ownership.c` covers that it is allowed) +overwrites that buffer in place, so a thread reading the name at that moment can +see a torn string. Every other registry operation is serialized; this one cannot +be, because the reader is outside the lock by the time it reads the characters. + +Documented in README.md and UPGRADING.md as "register names during +initialization". Closing it properly means making a registered name immutable — +either refusing a rename outright (a behavior change, and +`tests/err_name_ownership.c` asserts the current contract), or copying names +into a bump-allocated arena and publishing the pointer with a release store, so +a rename allocates new storage instead of rewriting live storage. The arena is +the better answer; it costs a second capacity limit and its exhaustion path. ## 6. Deprecate the two-argument name-registration path @@ -72,8 +85,9 @@ build whose tables are too small for the library's own entries, and both sizes are `PRIVATE` to the library target, so a test executable cannot set them. Closing it means a second library target built with tiny tables plus a -`WILL_FAIL` test linked against it — worth doing when the CMake gains a -sanitizer variant (item 1), since that adds the same machinery. +`WILL_FAIL` test linked against it. Nothing in the CMake does that yet: the +sanitizer and coverage options vary the *flags* of the one library target, not +its compile definitions. Related: branch coverage on `src/error.c` now sits just above its 50% gate. Every `FAIL_*` site carries about six branch outcomes of error-construction diff --git a/UPGRADING.md b/UPGRADING.md index b9f6a0a..d1f31e9 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,3 +1,100 @@ +# Upgrade notice: thread safety (2.0.0) + +2.0.0 makes the library thread safe. Every entry point may be called from any +thread, `akerr_init()` runs exactly once however many threads race into it, and +the error pool and the status registry are serialized. + +This is an ABI break. Rebuild libakerror and everything that includes its +header; the soname moved to `libakerror.so.2` so the two cannot be mixed by +accident. + +What moved at the ABI: + +* `__akerr_last_ignored` is thread-local storage. An ignored error is a fact + about the thread that ignored it, and one shared slot had two threads + overwriting each other's. The `IGNORE` macro expands at *your* call site, so + your objects reference the symbol under whichever storage model your header + said — which is why this cannot be mixed. +* `akerr_next_error()` returns a context that **already holds one reference**. + Finding a free slot and claiming it has to be one operation under the pool + lock, or two threads scanning at once are handed the same slot. + `ENSURE_ERROR_READY` therefore no longer increments the count. Code compiled + against a 1.x header and linked against 2.x would count every reference twice + and never return a slot to the pool. + +What changed in the API: nothing you call, unless you call `akerr_next_error()` +yourself. If you do, you now own a reference and must release it — the same +thing you were doing already if you were using the context for anything. + +New build options, both on libakerror itself: + +| Option | Default | Meaning | +| --- | --- | --- | +| `AKERR_THREADS` | `auto` | Threading backend: `auto`, `pthread`, or `none` | +| `AKERR_SANITIZE` | (empty) | Sanitizers for the library and its tests, e.g. `thread` | + +`auto` takes POSIX threads and **fails the configure** when it cannot find them, +rather than quietly building a library that reports itself thread safe and is +not. `-DAKERR_THREADS=none` is how you say you meant it: no locking, no +thread-local storage, undefined if you then use more than one thread. + +The generated header records which one you built, as `AKERR_THREAD_SAFE` (`1` or +`0`), so a consumer can test what it linked against and cannot disagree with the +library about it. + +## What thread safety here does and does not mean + +Safe from any thread, with no coordination on your part: + +* Raising, catching, handling, passing, ignoring and releasing errors. +* `akerr_reserve_status_range()` and `akerr_register_status_name()`. Two threads + reserving overlapping ranges cannot both succeed: one gets `NULL`, the other + gets `AKERR_STATUS_RANGE_OVERLAP` naming the winner. +* `akerr_name_for_status(status, NULL)` lookups, concurrently with each other + and with registrations of *other* statuses. +* `akerr_init()`, from any number of threads at once. + +Still yours to coordinate: + +* **One error context is owned by one thread.** The library hands it to the + thread that raised it. Handing it to another thread is your synchronization. +* **`akerr_log_method` and `akerr_handler_unhandled_error`** are read on every + error and written by nobody but you. Set them during startup, before spawning. +* **Renaming a status while another thread looks it up.** + `akerr_name_for_status()` returns a pointer into the registry — stable for the + life of the process, which is what makes it usable from a stack trace — and + registering a second name for the same status overwrites that buffer in place. + Register names during initialization. This is the one registry operation the + lock cannot make safe, because the reader is outside the lock by the time it + reads the string. +* **Which unhandled error wins.** Two threads reaching `FINISH_NORETURN` with + unhandled errors at the same instant both print a complete stack trace (the + buffer belongs to the context, and each line is a single `akerr_log_method` + call) and both call `akerr_handler_unhandled_error`. The process exits with + whichever status got there first. + +## Cost + +One recursive lock covers both the pool and the registry, so error +*construction* is serialized process-wide. Errors are the exceptional path and +correctness there is worth more than throughput, but a program that raises +errors in a hot loop will feel it. + +The per-thread last-ditch context is a whole `akerr_ErrorContext` (tens of +kilobytes) in thread-local storage, allocated per thread on first use of the +library from that thread. + +## Proving it + +`tests/err_threads_init.c`, `tests/err_threads_pool.c` and +`tests/err_threads_registry.c` assert the properties directly and run in the +normal suite. The run that proves there is no data race underneath them is +ThreadSanitizer: + +```sh +scripts/thread_test.sh +``` + # Upgrade notice: custom status codes (1.0.0) Version 1.0.0 replaces the consumer-sized status-name array with a private @@ -202,7 +299,7 @@ Exhausting either table raises an error to the caller; it is never silent. ## Thread safety -The registry is process-global mutable state with no locking. Reserve ranges and -register names during single-threaded initialization, before spawning threads. -Lookups (`akerr_name_for_status(status, NULL)`) are safe to call concurrently -once registration has finished. +As of 2.0.0 the registry is serialized: reservations, registrations and lookups +are all safe to call concurrently. See "What thread safety here does and does +not mean" at the top of this document for the two things that are still yours to +coordinate. diff --git a/cmake/thread_safe.stamp.in b/cmake/thread_safe.stamp.in new file mode 100644 index 0000000..9619397 --- /dev/null +++ b/cmake/thread_safe.stamp.in @@ -0,0 +1 @@ +@AKERR_THREAD_SAFE@ diff --git a/include/akerror.tmpl.h b/include/akerror.tmpl.h index 60e0953..2df9f8b 100644 --- a/include/akerror.tmpl.h +++ b/include/akerror.tmpl.h @@ -9,6 +9,42 @@ #include #endif +/* + * Threading. + * + * scripts/generrno.sh stamps this value in at build time from the AKERR_THREADS + * build option, the same way it stamps AKERR_LAST_ERRNO_VALUE. It is generated + * rather than defined by the consumer on purpose: whether the library + * serializes its global state and whether __akerr_last_ignored is a + * thread-local are the same decision, and a consumer that disagreed with the + * library about it would link against a differently shaped symbol. + * + * 1 The error pool and the status registry are mutex protected, and the + * per-thread state below is thread local. Every entry point may be called + * from any thread. See "Thread safety" in README.md for what that does and + * does not cover. + * 0 The library was built -DAKERR_THREADS=none for a single-threaded + * process: no locking, no thread-local storage, and calling it from more + * than one thread is undefined. + * + * Consumers can test it: #if AKERR_THREAD_SAFE. + */ +#define AKERR_THREAD_SAFE AKERR_THREAD_SAFE_SED + +#if AKERR_THREAD_SAFE == 1 +#if defined(__GNUC__) || defined(__clang__) +#define AKERR_THREAD_LOCAL __thread +#elif defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L +#define AKERR_THREAD_LOCAL _Thread_local +#elif defined(_MSC_VER) +#define AKERR_THREAD_LOCAL __declspec(thread) +#else +#error "libakerror was built thread safe, but this compiler has no thread-local storage specifier that akerror.h knows about. Rebuild libakerror with -DAKERR_THREADS=none, or add the spelling here." +#endif +#else +#define AKERR_THREAD_LOCAL +#endif + // FIXME: This is huge now. It used to be 1000 bytes, then I wanted to report errors // related to filesystem paths, which made it grow beyond PATH_MAX, then I started // reporting messages including 2 file paths (PATH_MAX * 2), so now to make the compiler warnings @@ -101,11 +137,29 @@ typedef void (*akerr_ErrorUnhandledErrorHandler)(akerr_ErrorContext *errctx); typedef void (*akerr_ErrorLogFunction)(const char *f, ...); extern akerr_ErrorContext AKERR_ARRAY_ERROR[AKERR_MAX_ARRAY_ERROR]; +/* + * Set these before starting threads. They are read on every error and written + * by nothing but your own code, so changing one while other threads are raising + * errors is a data race the library cannot mediate. + */ extern akerr_ErrorUnhandledErrorHandler akerr_handler_unhandled_error; extern akerr_ErrorLogFunction akerr_log_method; -extern akerr_ErrorContext *__akerr_last_ignored; +/* + * The error IGNORE() last swallowed, per thread: an ignored error is a fact + * about the thread that ignored it, and one shared slot would have two threads + * overwriting each other's. Thread local only when AKERR_THREAD_SAFE is 1. + */ +extern AKERR_THREAD_LOCAL akerr_ErrorContext *__akerr_last_ignored; akerr_ErrorContext AKERR_NOIGNORE *akerr_release_error(akerr_ErrorContext *ptr); +/* + * Check a context out of the pool. The returned context already carries one + * reference: finding a free slot and claiming it is a single operation under + * the pool lock, because two threads scanning at once would otherwise be handed + * the same slot. Release it with akerr_release_error() (or let RELEASE_ERROR, + * SUCCEED_RETURN or FINISH do it for you). Returns NULL when every slot is + * checked out. + */ akerr_ErrorContext AKERR_NOIGNORE *akerr_next_error(); /* * Look up (name == NULL) or register (name != NULL) the display name for a @@ -173,6 +227,11 @@ akerr_ErrorContext AKERR_NOIGNORE *__akerr_copy_string(char *destination, int ca akerr_init(); \ akerr_ErrorContext __attribute__ ((unused)) *__err_context = NULL; +/* + * akerr_next_error() hands back a context that already holds one reference -- + * it has to, or a second thread could be given the same slot between the scan + * and the increment. There is nothing to increment here. + */ #define ENSURE_ERROR_READY(__err_context) \ if ( __err_context == NULL ) { \ __err_context = akerr_next_error(); \ @@ -180,7 +239,6 @@ akerr_ErrorContext AKERR_NOIGNORE *__akerr_copy_string(char *destination, int ca akerr_log_method("%s:%s:%d: Unable to pull an error context from the array!", __FILE__, (char *)__func__, __LINE__); \ exit(1); \ } \ - __err_context->refcount += 1; \ } /* diff --git a/scripts/generrno.sh b/scripts/generrno.sh index b16d8a8..9eebcab 100644 --- a/scripts/generrno.sh +++ b/scripts/generrno.sh @@ -2,6 +2,16 @@ srcdir=$1 outdir=$2 +# 1 when the library was configured with a threading backend, 0 for +# -DAKERR_THREADS=none. Stamped into the header so a consumer cannot disagree +# with the library about whether it locks and whether its per-thread state is +# thread local. Defaults to 1 for a hand-run of this script. +thread_safe=${3:-1} + +if [ "${thread_safe}" != "0" ] && [ "${thread_safe}" != "1" ]; then + echo "$0: thread-safe argument must be 0 or 1, got '${thread_safe}'" >&2 + exit 1 +fi mkdir -p ${outdir}/src mkdir -p ${outdir}/include @@ -28,4 +38,6 @@ errno --list | while read LINE; do echo " __akerr_name_library_status(${define}, \"${desc}\");" >> ${outdir}/src/errno.c ; done; echo "}" >> ${outdir}/src/errno.c -sed "s/#define AKERR_LAST_ERRNO_VALUE .*/#define AKERR_LAST_ERRNO_VALUE ${maxval}/" ${srcdir}/include/akerror.tmpl.h > ${outdir}/include/akerror.h +sed -e "s/#define AKERR_LAST_ERRNO_VALUE .*/#define AKERR_LAST_ERRNO_VALUE ${maxval}/" \ + -e "s/#define AKERR_THREAD_SAFE .*/#define AKERR_THREAD_SAFE ${thread_safe}/" \ + ${srcdir}/include/akerror.tmpl.h > ${outdir}/include/akerror.h diff --git a/scripts/thread_test.sh b/scripts/thread_test.sh new file mode 100755 index 0000000..6d336f5 --- /dev/null +++ b/scripts/thread_test.sh @@ -0,0 +1,45 @@ +#!/bin/bash +# +# Run the test suite under ThreadSanitizer. +# +# The thread tests assert properties -- exclusive ownership of pool slots and of +# reserved status ranges -- that hold or fail without any tooling. This is the +# run that proves the absence of a data race underneath them, so it is the one +# that has to be easy to type. +# +# Usage: scripts/thread_test.sh [build directory] + +set -o errexit +set -o nounset +set -o pipefail + +BUILD_DIR=${1:-build/tsan} +SANITIZE=${AKERR_SANITIZE:-thread} + +# Work from the repository root whatever directory this was invoked from, so a +# relative build directory always lands in the same place. +cd "$(dirname "$0")/.." + +for tool in cmake ctest; do + if ! command -v "${tool}" >/dev/null 2>&1; then + echo "$0: ${tool} is required and was not found" >&2 + exit 1 + fi +done + +# ThreadSanitizer maps its shadow memory at fixed addresses and aborts with +# "FATAL: ThreadSanitizer: unexpected memory mapping" on kernels configured for +# more ASLR entropy than it allows (vm.mmap_rnd_bits > 28, the default on +# several recent distributions). Running with ASLR disabled sidesteps it without +# needing root, which "sysctl -w vm.mmap_rnd_bits=28" would. +RUNNER=() +if command -v setarch >/dev/null 2>&1; then + RUNNER=(setarch -R) +else + echo "$0: setarch not found; if ThreadSanitizer aborts with an unexpected" \ + "memory mapping, lower vm.mmap_rnd_bits to 28" >&2 +fi + +cmake -S . -B "${BUILD_DIR}" -DAKERR_SANITIZE="${SANITIZE}" +cmake --build "${BUILD_DIR}" +"${RUNNER[@]}" ctest --test-dir "${BUILD_DIR}" --output-on-failure "${@:2}" diff --git a/src/error.c b/src/error.c index 326e880..2e5d311 100644 --- a/src/error.c +++ b/src/error.c @@ -1,15 +1,44 @@ #include "akerror.h" +#include "lock.h" #if defined(AKERR_USE_STDLIB) && AKERR_USE_STDLIB == 1 #include #include #include #endif // AKERR_USE_STDLIB -akerr_ErrorContext __akerr_last_ditch; -akerr_ErrorContext *__akerr_last_ignored; +/* + * Per-thread state. + * + * The last-ditch context is where a failure gets reported when there is no pool + * slot to report it from -- akerr_release_error(NULL). One shared copy would + * have two threads formatting a message into the same buffer, so each thread + * gets its own. Thread-local storage is zero initialized, which is why + * akerr_last_ditch_context() below sets the stack-trace cursor lazily rather + * than akerr_init() setting it for everyone: akerr_init() runs on one thread + * and cannot reach the others' copies. + * + * It is not small (an akerr_ErrorContext is tens of kilobytes), but the storage + * is allocated per thread only when that thread first touches the library's + * thread-local block, and the alternative is a shared buffer that two threads + * can be writing at once. + */ +static AKERR_THREAD_LOCAL akerr_ErrorContext __akerr_last_ditch; +AKERR_THREAD_LOCAL akerr_ErrorContext *__akerr_last_ignored; akerr_ErrorUnhandledErrorHandler akerr_handler_unhandled_error; akerr_ErrorLogFunction akerr_log_method = NULL; +/* + * One recursive lock over both the error pool and the status registry. See + * src/lock.h for why it is one lock, and why it is recursive. + * + * Everything below that touches either table does so through a function whose + * name ends in _locked, called from a wrapper that takes the lock and releases + * it on the single return path. The wrappers exist because the locked bodies + * are written with the FAIL_*_RETURN macros, which return from the middle of a + * function -- so those bodies cannot be the ones holding the lock. + */ +static akerr_Mutex akerr_state_lock; + /* * Status-name registry. * @@ -97,6 +126,9 @@ akerr_ErrorContext *__akerr_copy_string(char *destination, int capacity, * A range test also accepts pointers into the *interior* of an element, which * would then be treated as the head of an akerr_ErrorContext and written * through. Keep this an element-wise scan; it is not a missed optimization. + * + * Takes no lock: the addresses of the pool slots are fixed for the life of the + * process, and nothing here reads a slot's contents. */ int akerr_valid_error_address(akerr_ErrorContext *ptr) { @@ -156,77 +188,121 @@ void __akerr_name_library_status(int status, const char *name) } /* - * Idempotent. `inited` is set before any work so that the registry calls below - * -- and the public registry entry points, which all call akerr_init() so that - * a consumer reserving its range before anything else touches the library - * cannot have that reservation wiped by a later first-use of the pool -- see - * themselves as already initialized instead of recursing. + * The calling thread's last-ditch context. + * + * Thread-local storage starts zeroed, so a NULL stack-trace cursor means this + * thread has not used its copy yet. Checking the cursor rather than a separate + * flag keeps the whole thing self-describing: the cursor is the one field that + * must not be zero for the context to be usable at all. + */ +static akerr_ErrorContext *akerr_last_ditch_context(void) +{ + if ( __akerr_last_ditch.stacktracebufptr == NULL ) { + memset((void *)&__akerr_last_ditch, 0x00, sizeof(akerr_ErrorContext)); + __akerr_last_ditch.stacktracebufptr = (char *)&__akerr_last_ditch.stacktracebuf; + } + return &__akerr_last_ditch; +} + +static AKERR_THREAD_LOCAL int akerr_initializing; +static akerr_Once akerr_state_once = AKERR_ONCE_INIT; + +/* + * Runs exactly once per process, under akerr_once(). Everything it calls -- + * the registry entry points, and the pool underneath them -- calls akerr_init() + * itself, so the first thing it does is raise the re-entry flag; see + * akerr_init() below. + * + * The lock is initialized before anything that could take it. That ordering is + * the reason this work lives in a once-routine instead of a + * check-a-flag-and-go: a second thread arriving while this one is still + * populating the tables must block until they are complete, not walk them. + */ +static void akerr_init_state(void) +{ + akerr_mutex_init(&akerr_state_lock); + akerr_initializing = 1; + + for (int i = 0; i < AKERR_MAX_ARRAY_ERROR; i++ ) { + memset((void *)&AKERR_ARRAY_ERROR[i], 0x00, sizeof(akerr_ErrorContext)); + AKERR_ARRAY_ERROR[i].arrayid = i; + AKERR_ARRAY_ERROR[i].stacktracebufptr = (char *)&AKERR_ARRAY_ERROR[i].stacktracebuf; + } + __akerr_last_ignored = NULL; + (void)akerr_last_ditch_context(); + if ( akerr_log_method == NULL ) { + akerr_log_method = &akerr_default_logger; + } + akerr_handler_unhandled_error = &akerr_default_handler_unhandled_error; + memset((void *)&akerr_status_names[0], 0x00, sizeof(akerr_status_names)); + memset((void *)&akerr_status_ranges[0], 0x00, sizeof(akerr_status_ranges)); + akerr_status_name_count = 0; + akerr_status_range_count = 0; + + /* errno and AKERR_* values are the library-owned compatibility band. + * This must precede every registration below: naming a status is only + * permitted inside a reserved range. Terminal for the same reason as + * __akerr_name_library_status(), and handled the same way: without this + * band the library owns nothing, so none of the names below could + * register either. */ + PREPARE_ERROR(errctx); + ATTEMPT { + CATCH(errctx, akerr_reserve_status_range(0, AKERR_RESERVED_STATUS_COUNT, + AKERR_LIBRARY_OWNER)); + } CLEANUP { + } PROCESS(errctx) { + } FINISH_NORETURN(errctx); + + /* Every AKERR_* code gets a name; tests/err_error_names.c asserts the + * list is exhaustive so a new code cannot be added without one. */ + __akerr_name_library_status(AKERR_NULLPOINTER, "Null Pointer Error"); + __akerr_name_library_status(AKERR_OUTOFBOUNDS, "Out Of Bounds Error"); + __akerr_name_library_status(AKERR_API, "API Error"); + __akerr_name_library_status(AKERR_ATTRIBUTE, "Attribute Error"); + __akerr_name_library_status(AKERR_TYPE, "Type Error"); + __akerr_name_library_status(AKERR_KEY, "Key Error"); + __akerr_name_library_status(AKERR_INDEX, "Index Error"); + __akerr_name_library_status(AKERR_FORMAT, "Format Error"); + __akerr_name_library_status(AKERR_IO, "Input Output Error"); + __akerr_name_library_status(AKERR_VALUE, "Value Error"); + __akerr_name_library_status(AKERR_RELATIONSHIP, "Relationship Error"); + __akerr_name_library_status(AKERR_EOF, "End Of File"); + __akerr_name_library_status(AKERR_CIRCULAR_REFERENCE, "Circular Reference Error"); + __akerr_name_library_status(AKERR_ITERATOR_BREAK, "Iterator Break"); + __akerr_name_library_status(AKERR_NOT_IMPLEMENTED, "Not Implemented"); + __akerr_name_library_status(AKERR_BADEXC, "Invalid akerr_ErrorContext"); + __akerr_name_library_status(AKERR_STATUS_RANGE_OVERLAP, "Status Range Overlap"); + __akerr_name_library_status(AKERR_STATUS_RANGE_FULL, "Status Range Table Full"); + __akerr_name_library_status(AKERR_STATUS_RANGE_INVALID, "Invalid Status Range"); + __akerr_name_library_status(AKERR_STATUS_NAME_UNRESERVED, "Unreserved Status Name"); + __akerr_name_library_status(AKERR_STATUS_NAME_FOREIGN, "Foreign Status Name"); + __akerr_name_library_status(AKERR_STATUS_NAME_FULL, "Status Name Registry Full"); + __akerr_name_library_status(AKERR_STATUS_NAME_INVALID, "Invalid Status Name"); +#if (defined(AKERR_USE_STDLIB) && AKERR_USE_STDLIB == 1) || (!defined(AKERR_USE_STDLIB)) + akerr_init_errno(); +#endif + + akerr_initializing = 0; +} + +/* + * Idempotent, and safe to call from any thread at any time. Every public entry + * point calls it -- so that a consumer reserving its range before anything else + * touches the library cannot have that reservation wiped by a later first-use + * of the pool -- which means it is also on the path of everything + * akerr_init_state() itself calls. + * + * The re-entry guard is thread local, and has to be: only the thread running + * the once-routine may skip past it. A second thread arriving mid-initialization + * must block inside akerr_once() until the tables are complete, which a shared + * flag set at the top of initialization would have let it walk right past. */ void akerr_init() { - static int inited = 0; - if ( inited == 0 ) { - inited = 1; - for (int i = 0; i < AKERR_MAX_ARRAY_ERROR; i++ ) { - memset((void *)&AKERR_ARRAY_ERROR[i], 0x00, sizeof(akerr_ErrorContext)); - AKERR_ARRAY_ERROR[i].arrayid = i; - AKERR_ARRAY_ERROR[i].stacktracebufptr = (char *)&AKERR_ARRAY_ERROR[i].stacktracebuf; - } - __akerr_last_ignored = NULL; - memset((void *)&__akerr_last_ditch, 0x00, sizeof(akerr_ErrorContext)); - __akerr_last_ditch.stacktracebufptr = (char *)&__akerr_last_ditch.stacktracebuf; - if ( akerr_log_method == NULL ) { - akerr_log_method = &akerr_default_logger; - } - akerr_handler_unhandled_error = &akerr_default_handler_unhandled_error; - memset((void *)&akerr_status_names[0], 0x00, sizeof(akerr_status_names)); - memset((void *)&akerr_status_ranges[0], 0x00, sizeof(akerr_status_ranges)); - akerr_status_name_count = 0; - akerr_status_range_count = 0; - - /* errno and AKERR_* values are the library-owned compatibility band. - * This must precede every registration below: naming a status is only - * permitted inside a reserved range. Terminal for the same reason as - * __akerr_name_library_status(), and handled the same way: without this - * band the library owns nothing, so none of the names below could - * register either. */ - PREPARE_ERROR(errctx); - ATTEMPT { - CATCH(errctx, akerr_reserve_status_range(0, AKERR_RESERVED_STATUS_COUNT, - AKERR_LIBRARY_OWNER)); - } CLEANUP { - } PROCESS(errctx) { - } FINISH_NORETURN(errctx); - - /* Every AKERR_* code gets a name; tests/err_error_names.c asserts the - * list is exhaustive so a new code cannot be added without one. */ - __akerr_name_library_status(AKERR_NULLPOINTER, "Null Pointer Error"); - __akerr_name_library_status(AKERR_OUTOFBOUNDS, "Out Of Bounds Error"); - __akerr_name_library_status(AKERR_API, "API Error"); - __akerr_name_library_status(AKERR_ATTRIBUTE, "Attribute Error"); - __akerr_name_library_status(AKERR_TYPE, "Type Error"); - __akerr_name_library_status(AKERR_KEY, "Key Error"); - __akerr_name_library_status(AKERR_INDEX, "Index Error"); - __akerr_name_library_status(AKERR_FORMAT, "Format Error"); - __akerr_name_library_status(AKERR_IO, "Input Output Error"); - __akerr_name_library_status(AKERR_VALUE, "Value Error"); - __akerr_name_library_status(AKERR_RELATIONSHIP, "Relationship Error"); - __akerr_name_library_status(AKERR_EOF, "End Of File"); - __akerr_name_library_status(AKERR_CIRCULAR_REFERENCE, "Circular Reference Error"); - __akerr_name_library_status(AKERR_ITERATOR_BREAK, "Iterator Break"); - __akerr_name_library_status(AKERR_NOT_IMPLEMENTED, "Not Implemented"); - __akerr_name_library_status(AKERR_BADEXC, "Invalid akerr_ErrorContext"); - __akerr_name_library_status(AKERR_STATUS_RANGE_OVERLAP, "Status Range Overlap"); - __akerr_name_library_status(AKERR_STATUS_RANGE_FULL, "Status Range Table Full"); - __akerr_name_library_status(AKERR_STATUS_RANGE_INVALID, "Invalid Status Range"); - __akerr_name_library_status(AKERR_STATUS_NAME_UNRESERVED, "Unreserved Status Name"); - __akerr_name_library_status(AKERR_STATUS_NAME_FOREIGN, "Foreign Status Name"); - __akerr_name_library_status(AKERR_STATUS_NAME_FULL, "Status Name Registry Full"); - __akerr_name_library_status(AKERR_STATUS_NAME_INVALID, "Invalid Status Name"); -#if (defined(AKERR_USE_STDLIB) && AKERR_USE_STDLIB == 1) || (!defined(AKERR_USE_STDLIB)) - akerr_init_errno(); -#endif + if ( akerr_initializing != 0 ) { + return; } + akerr_once(&akerr_state_once, &akerr_init_state); } void akerr_default_handler_unhandled_error(akerr_ErrorContext *errctx) @@ -237,23 +313,46 @@ void akerr_default_handler_unhandled_error(akerr_ErrorContext *errctx) exit(errctx->status); } +/* + * Claim the lowest free slot. Finding it and taking the reference are one + * operation under the lock: a scan that returned an unclaimed slot would hand + * the same one to every thread that scanned before the first of them got around + * to incrementing the count. + */ akerr_ErrorContext *akerr_next_error() { + akerr_ErrorContext *found = (akerr_ErrorContext *)NULL; + + akerr_init(); + akerr_mutex_lock(&akerr_state_lock); for (int i = 0; i < AKERR_MAX_ARRAY_ERROR; i++ ) { if ( AKERR_ARRAY_ERROR[i].refcount == 0 ) { - return &AKERR_ARRAY_ERROR[i]; + found = &AKERR_ARRAY_ERROR[i]; + found->refcount = 1; + break; } } - return (akerr_ErrorContext *)NULL; + akerr_mutex_unlock(&akerr_state_lock); + return found; } +/* + * The wipe returns the slot to the pool, so it and the decrement that triggers + * it are one operation under the lock. Otherwise a thread that saw the count + * reach zero could be handed the slot by akerr_next_error() and start writing + * its error into it while the releasing thread was still memsetting it. + */ akerr_ErrorContext *akerr_release_error(akerr_ErrorContext *err) { int oldid = 0; + akerr_ErrorContext *remaining = err; + + akerr_init(); if ( err == NULL ) { - akerr_ErrorContext *errctx = &__akerr_last_ditch; + akerr_ErrorContext *errctx = akerr_last_ditch_context(); FAIL_RETURN(errctx, AKERR_NULLPOINTER, "akerr_release_error got NULL context pointer"); } + akerr_mutex_lock(&akerr_state_lock); if ( err->refcount > 0 ) { err->refcount -= 1; } @@ -262,9 +361,10 @@ akerr_ErrorContext *akerr_release_error(akerr_ErrorContext *err) memset(err, 0x00, sizeof(akerr_ErrorContext)); err->stacktracebufptr = (char *)&err->stacktracebuf; err->arrayid = oldid; - return NULL; + remaining = NULL; } - return err; + akerr_mutex_unlock(&akerr_state_lock); + return remaining; } @@ -288,7 +388,7 @@ static unsigned akerr_status_hash(int status) /* * Find the slot holding `status`. With create != 0, claim a free slot for it if * it is not present yet. Returns NULL when the status is absent and either no - * slot was requested or the registry is full. + * slot was requested or the registry is full. Caller holds akerr_state_lock. * * The `& (AKERR_STATUS_NAME_SLOTS - 1)` below is load-bearing and fails * silently: off by one in either direction and the probe indexes past @@ -328,7 +428,8 @@ static akerr_StatusName *akerr_status_slot(int status, int create) return NULL; } -/* The reservation covering `status`, or NULL if nobody has claimed it. */ +/* The reservation covering `status`, or NULL if nobody has claimed it. Caller + * holds akerr_state_lock. */ static akerr_StatusRange *akerr_range_for_status(int status) { for ( int i = 0; i < akerr_status_range_count; i++ ) { @@ -348,10 +449,14 @@ static akerr_StatusRange *akerr_range_for_status(int status) * fails to register degrades into "Unknown Error" in stack traces, which is * exactly the kind of quiet loss this registry exists to prevent -- so the * message carries everything a caller needs to see in a stack trace. + * + * Caller holds akerr_state_lock. The FAIL_* macros below re-enter the library + * to build their error -- a pool slot from akerr_next_error(), and a status + * name for the stack trace -- and that re-entry is why the lock is recursive. */ -static akerr_ErrorContext AKERR_NOIGNORE *akerr_store_status_name(const char *owner, - int status, - const char *name) +static akerr_ErrorContext AKERR_NOIGNORE *akerr_store_status_name_locked(const char *owner, + int status, + const char *name) { akerr_StatusRange *range; akerr_StatusName *entry; @@ -394,12 +499,16 @@ static akerr_ErrorContext AKERR_NOIGNORE *akerr_store_status_name(const char *ow /* * Register a name for a status inside a range the caller reserved. Both strings - * are checked here rather than only inside akerr_store_status_name(): the store - * accepts a NULL owner for the legacy akerr_name_for_status() path, so a NULL - * arriving through *this* entry point would be read as "caller did not identify - * itself" and skip the ownership check entirely. + * are checked here rather than only inside akerr_store_status_name_locked(): the + * store accepts a NULL owner for the legacy akerr_name_for_status() path, so a + * NULL arriving through *this* entry point would be read as "caller did not + * identify itself" and skip the ownership check entirely. + * + * Caller holds akerr_state_lock. */ -akerr_ErrorContext *akerr_register_status_name(const char *owner, int status, const char *name) +static akerr_ErrorContext AKERR_NOIGNORE *akerr_register_status_name_locked(const char *owner, + int status, + const char *name) { PREPARE_ERROR(errctx); @@ -410,10 +519,21 @@ akerr_ErrorContext *akerr_register_status_name(const char *owner, int status, co FAIL_NONZERO_RETURN(errctx, (name == NULL), AKERR_STATUS_NAME_INVALID, "Refusing to name status %d for %s: the name is NULL", status, owner); - PASS(errctx, akerr_store_status_name(owner, status, name)); + PASS(errctx, akerr_store_status_name_locked(owner, status, name)); SUCCEED_RETURN(errctx); } +akerr_ErrorContext *akerr_register_status_name(const char *owner, int status, const char *name) +{ + akerr_ErrorContext *errctx; + + akerr_init(); + akerr_mutex_lock(&akerr_state_lock); + errctx = akerr_register_status_name_locked(owner, status, name); + akerr_mutex_unlock(&akerr_state_lock); + return errctx; +} + /* * Return or set a name. Status magnitude is unrelated to storage size. * @@ -426,16 +546,40 @@ akerr_ErrorContext *akerr_register_status_name(const char *owner, int status, co * The lookup path (name == NULL) deliberately stays clear of all of this. FAIL * calls it to render a status into a stack trace, so it must not itself need an * error context. + * + * The name is returned by pointer into the registry, which never resizes and + * never removes an entry, so the pointer is good for the life of the process. + * Its *contents* are stable as long as nobody registers a second name for the + * same status: a rename overwrites the buffer in place, and a lookup on another + * thread can be reading it. Register names during initialization -- renaming a + * live status while other threads run is the one registry operation the lock + * cannot make safe, because the reader is outside it by then. */ +static akerr_ErrorContext AKERR_NOIGNORE *akerr_store_status_name(const char *owner, + int status, + const char *name) +{ + akerr_ErrorContext *errctx; + + akerr_mutex_lock(&akerr_state_lock); + errctx = akerr_store_status_name_locked(owner, status, name); + akerr_mutex_unlock(&akerr_state_lock); + return errctx; +} + char *akerr_name_for_status(int status, char *name) { akerr_StatusName *entry; + char *found = "Unknown Error"; akerr_init(); if ( name != NULL ) { PREPARE_ERROR(errctx); int refused = 0; + /* The store takes and releases the lock itself, so the handler below + * calls akerr_log_method -- consumer code, which may do anything at all + * including calling back into this library -- without holding it. */ ATTEMPT { CATCH(errctx, akerr_store_status_name(NULL, status, name)); } CLEANUP { @@ -449,15 +593,22 @@ char *akerr_name_for_status(int status, char *name) return "Unknown Error"; } } + akerr_mutex_lock(&akerr_state_lock); entry = akerr_status_slot(status, 0); - if ( entry == NULL ) { - return "Unknown Error"; + if ( entry != NULL ) { + found = entry->name; } - return entry->name; + akerr_mutex_unlock(&akerr_state_lock); + return found; } -/* Reserve an inclusive status interval and reject collisions. */ -akerr_ErrorContext *akerr_reserve_status_range(int first_status, int count, const char *owner) +/* Reserve an inclusive status interval and reject collisions. Caller holds + * akerr_state_lock: the overlap scan and the entry that follows it are one + * decision, so two threads claiming overlapping ranges at once must not be able + * to both find the table clear. */ +static akerr_ErrorContext AKERR_NOIGNORE *akerr_reserve_status_range_locked(int first_status, + int count, + const char *owner) { int last_status; PREPARE_ERROR(errctx); @@ -510,3 +661,14 @@ akerr_ErrorContext *akerr_reserve_status_range(int first_status, int count, cons akerr_status_range_count++; SUCCEED_RETURN(errctx); } + +akerr_ErrorContext *akerr_reserve_status_range(int first_status, int count, const char *owner) +{ + akerr_ErrorContext *errctx; + + akerr_init(); + akerr_mutex_lock(&akerr_state_lock); + errctx = akerr_reserve_status_range_locked(first_status, count, owner); + akerr_mutex_unlock(&akerr_state_lock); + return errctx; +} diff --git a/src/lock.h b/src/lock.h new file mode 100644 index 0000000..b366590 --- /dev/null +++ b/src/lock.h @@ -0,0 +1,121 @@ +#ifndef _AKERR_LOCK_H_ +#define _AKERR_LOCK_H_ + +/* + * Serialization for the library's process-global state: the error pool + * (AKERR_ARRAY_ERROR) and the status registry. Private to the library -- none + * of this appears in the installed header, so the backend is not part of the + * ABI and can be changed without touching a consumer. + * + * The backend is chosen at configure time by the AKERR_THREADS build option and + * never by autodetection here. A build that quietly decided it did not need + * locking is exactly the failure this has to prevent: it would produce a + * library that reports itself thread safe and is not. + * + * AKERR_THREADS_PTHREAD POSIX threads. + * AKERR_THREADS_NONE No locking at all, for a build that has declared + * itself single threaded (-DAKERR_THREADS=none). + * + * One lock covers both tables, and it is recursive. Both are deliberate: + * + * - Raising an error re-enters the library. FAIL() calls + * akerr_name_for_status() to render the status into the stack trace and + * ENSURE_ERROR_READY() to check a context out of the pool, so a refusal + * raised from inside a locked registry operation takes the lock again on + * the same thread. A non-recursive mutex deadlocks there. + * - With a single lock there is no lock ordering to get wrong, and no way for + * a future caller to acquire the pool and the registry in the opposite + * order from this file. + * + * The cost is that error *construction* is serialized across threads. Errors + * are the exceptional path; correctness is worth more there than throughput. + */ + +/* + * PTHREAD_MUTEX_RECURSIVE is XSI, so glibc hides it under a strict -std=c99 + * without _XOPEN_SOURCE. No feature-test macro is defined here, because the + * public header already needs the same one for PATH_MAX: a build strict enough + * to lose one has already lost the other. Build with -D_XOPEN_SOURCE=700 if you + * need strict C99. + */ +#if defined(AKERR_THREADS_PTHREAD) && AKERR_THREADS_PTHREAD == 1 + +#include +#include + +typedef pthread_mutex_t akerr_Mutex; +typedef pthread_once_t akerr_Once; +#define AKERR_ONCE_INIT PTHREAD_ONCE_INIT + +/* + * Terminal on failure. There is no error context to raise into: the pool one + * would come from is the thing this lock protects, and every path that could + * report the failure needs the lock to do it. A process whose error library + * silently stopped locking is worse than one that stops here. + */ +static void akerr_mutex_init(akerr_Mutex *mutex) +{ + pthread_mutexattr_t attr; + + if ( pthread_mutexattr_init(&attr) != 0 || + pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE) != 0 || + pthread_mutex_init(mutex, &attr) != 0 ) { + abort(); + } + pthread_mutexattr_destroy(&attr); +} + +static void akerr_mutex_lock(akerr_Mutex *mutex) +{ + pthread_mutex_lock(mutex); +} + +static void akerr_mutex_unlock(akerr_Mutex *mutex) +{ + pthread_mutex_unlock(mutex); +} + +static void akerr_once(akerr_Once *once, void (*routine)(void)) +{ + pthread_once(once, routine); +} + +#elif defined(AKERR_THREADS_NONE) && AKERR_THREADS_NONE == 1 + +typedef char akerr_Mutex; +typedef int akerr_Once; +#define AKERR_ONCE_INIT 0 + +static void akerr_mutex_init(akerr_Mutex *mutex) +{ + (void)mutex; +} + +static void akerr_mutex_lock(akerr_Mutex *mutex) +{ + (void)mutex; +} + +static void akerr_mutex_unlock(akerr_Mutex *mutex) +{ + (void)mutex; +} + +/* + * The flag is raised before the routine runs, so a routine that calls back into + * akerr_init() sees initialization already in progress and does not recurse -- + * the same short-circuit the pthread backend gets from akerr_initializing. + */ +static void akerr_once(akerr_Once *once, void (*routine)(void)) +{ + if ( *once == 0 ) { + *once = 1; + routine(); + } +} + +#else +#error "No threading backend selected. Build libakerror through its CMake, which defines AKERR_THREADS_PTHREAD or AKERR_THREADS_NONE from the AKERR_THREADS option." +#endif + +#endif // _AKERR_LOCK_H_ diff --git a/test.sh b/test.sh index cdab2e6..1c74390 100644 --- a/test.sh +++ b/test.sh @@ -1,5 +1,6 @@ cmake -S . -B build cmake --build build ctest --test-dir build --output-on-failure --output-junit "$(pwd)/ctest-junit.xml" + scripts/thread_test.sh build/tsan --output-junit "$(pwd)/tsan-junit.xml" python3 scripts/mutation_test.py --target src/error.c --junit mutation-junit.xml --threshold 65 python3 scripts/coverage.py --junit coverage-junit.xml --threshold 90 --branch-threshold 50 diff --git a/tests/err_pool_exhaust.c b/tests/err_pool_exhaust.c index 28ef594..d8e2226 100644 --- a/tests/err_pool_exhaust.c +++ b/tests/err_pool_exhaust.c @@ -15,11 +15,12 @@ int main(void) akerr_ErrorContext *slots[AKERR_MAX_ARRAY_ERROR]; - /* Check out every slot. */ + /* Check out every slot. Each arrives holding its own reference, so the + * next request cannot be handed the same one. */ for ( int i = 0; i < AKERR_MAX_ARRAY_ERROR; i++ ) { slots[i] = akerr_next_error(); AKERR_CHECK(slots[i] != NULL); - slots[i]->refcount = 1; + AKERR_CHECK(slots[i]->refcount == 1); } /* Pool is fully exhausted: the next request must fail cleanly. */ diff --git a/tests/err_release_refcount.c b/tests/err_release_refcount.c index 03a8cd4..caf2d6b 100644 --- a/tests/err_release_refcount.c +++ b/tests/err_release_refcount.c @@ -11,9 +11,10 @@ * - refcount == 0: releasing a context nobody holds must not underflow the * count; it wipes and returns NULL like any other fully-released slot. * - * The macro API never produces a refcount above 1 (ENSURE_ERROR_READY only - * increments when it acquires a fresh slot), so this test sets the count - * directly to model a caller that took an extra reference. + * The macro API never produces a refcount above 1 (akerr_next_error() takes the + * one reference a fresh slot gets), so this test sets the count directly to + * model a caller that took an extra reference, and clears it to model a slot + * nobody holds. */ int main(void) @@ -56,7 +57,8 @@ int main(void) */ akerr_ErrorContext *unheld = akerr_next_error(); AKERR_CHECK(unheld != NULL); - AKERR_CHECK(unheld->refcount == 0); + AKERR_CHECK(unheld->refcount == 1); + unheld->refcount = 0; ret = akerr_release_error(unheld); AKERR_CHECK(ret == NULL); AKERR_CHECK(unheld->refcount == 0); diff --git a/tests/err_threads.h b/tests/err_threads.h new file mode 100644 index 0000000..2568ecf --- /dev/null +++ b/tests/err_threads.h @@ -0,0 +1,149 @@ +#ifndef AKERR_TEST_THREADS_H +#define AKERR_TEST_THREADS_H + +/* + * Shared helpers for the thread-safety tests. + * + * These tests are checkable on their own -- they assert exclusive ownership of + * pool slots and of reserved ranges, which is a property, not a symptom -- but + * the run that proves the absence of a data race is the one under + * ThreadSanitizer: + * + * cmake -S . -B build/tsan -DAKERR_SANITIZE=thread + * cmake --build build/tsan + * ctest --test-dir build/tsan --output-on-failure + * + * Everything shared between the threads here is either read-only after the + * threads start, or touched through __atomic builtins. Anything else would be a + * race in the *test*, and TSan cannot tell whose bug it is reporting. + * + * A test body runs on AKERR_TEST_THREADS threads that meet at a barrier first, + * so they arrive at the library together instead of in start-up order. Failed + * checks are counted per thread rather than returned early: a thread that + * abandoned its work would leave the others holding pool slots and turn one + * failure into a cascade of unrelated ones. + */ + +#include "akerror.h" +#include +#include + +#define AKERR_TEST_THREADS 8 + +typedef struct +{ + int id; /* 1-based: 0 means "no thread" below */ + int failures; + pthread_barrier_t *barrier; +} akerr_ThreadArg; + +#define AKERR_TCHECK(__arg, __cond) \ + do { \ + if ( !(__cond) ) { \ + fprintf(stderr, "CHECK FAILED (thread %d): %s at %s:%d\n", \ + (__arg)->id, #__cond, __FILE__, __LINE__); \ + (__arg)->failures += 1; \ + } \ + } while ( 0 ) + +/* + * A logger that counts instead of printing. The capturing logger in + * err_capture.h appends to a shared buffer with a shared length, which is a + * data race the moment two threads log at once; these tests need a logger that + * is safe to install before spawning and still shows that something was + * reported. + */ +static int akerr_thread_log_count; + +static void __attribute__((unused)) akerr_thread_logger(const char *fmt, ...) +{ + (void)fmt; + __atomic_fetch_add(&akerr_thread_log_count, 1, __ATOMIC_RELAXED); +} + +static int __attribute__((unused)) akerr_thread_logs(void) +{ + return __atomic_load_n(&akerr_thread_log_count, __ATOMIC_RELAXED); +} + +/* + * Independent bookkeeping of who holds which pool slot. The library's own + * refcount says a slot is checked out; this says which thread it was checked + * out to, which is the part a racing akerr_next_error() would get wrong by + * handing one slot to two threads at once. + */ +static int akerr_slot_owner[AKERR_MAX_ARRAY_ERROR]; + +/* Returns 0 on success, or the id of the thread that already holds the slot. */ +static int __attribute__((unused)) akerr_slot_claim(int slot, int id) +{ + int unowned = 0; + + if ( __atomic_compare_exchange_n(&akerr_slot_owner[slot], &unowned, id, 0, + __ATOMIC_ACQ_REL, __ATOMIC_ACQUIRE) ) { + return 0; + } + return unowned; +} + +static int __attribute__((unused)) akerr_slot_holder(int slot) +{ + return __atomic_load_n(&akerr_slot_owner[slot], __ATOMIC_ACQUIRE); +} + +/* + * Give the slot up *before* releasing the error context. The other order hands + * the slot back to the pool while this thread still claims it, and the next + * thread to be given it reports a violation that is the test's fault. + */ +static void __attribute__((unused)) akerr_slot_drop(int slot) +{ + __atomic_store_n(&akerr_slot_owner[slot], 0, __ATOMIC_RELEASE); +} + +/* + * Run `body` on AKERR_TEST_THREADS threads and return the total number of + * failed checks. A thread that cannot be created is itself a failure, but the + * ones already running still get joined. + */ +static int __attribute__((unused)) akerr_run_threads(void *(*body)(void *)) +{ + pthread_t threads[AKERR_TEST_THREADS]; + akerr_ThreadArg args[AKERR_TEST_THREADS]; + pthread_barrier_t barrier; + int started = 0; + int failures = 0; + + if ( pthread_barrier_init(&barrier, NULL, AKERR_TEST_THREADS) != 0 ) { + fprintf(stderr, "CHECK FAILED: pthread_barrier_init at %s:%d\n", + __FILE__, __LINE__); + return 1; + } + for ( int i = 0; i < AKERR_TEST_THREADS; i++ ) { + args[i].id = i + 1; + args[i].failures = 0; + args[i].barrier = &barrier; + if ( pthread_create(&threads[i], NULL, body, &args[i]) != 0 ) { + fprintf(stderr, "CHECK FAILED: pthread_create for thread %d" + " at %s:%d\n", i + 1, __FILE__, __LINE__); + failures++; + break; + } + started++; + } + /* An unstarted thread never reaches the barrier, so the started ones would + * wait for it forever. Nothing to do but say so before hanging is diagnosed + * as a deadlock in the library. */ + if ( started != AKERR_TEST_THREADS ) { + fprintf(stderr, "only %d of %d threads started; the barrier will not" + " release\n", started, AKERR_TEST_THREADS); + } + for ( int i = 0; i < started; i++ ) { + pthread_join(threads[i], NULL); + failures += args[i].failures; + } + pthread_barrier_destroy(&barrier); + return failures; +} + +#endif // AKERR_TEST_THREADS_H diff --git a/tests/err_threads_init.c b/tests/err_threads_init.c new file mode 100644 index 0000000..6fef06c --- /dev/null +++ b/tests/err_threads_init.c @@ -0,0 +1,124 @@ +#include "akerror.h" +#include "err_capture.h" +#include "err_threads.h" +#include +#include + +/* + * akerr_init() runs exactly once, no matter how many threads reach the library + * at the same instant. + * + * This is the hardest of the three to get right, because initialization is + * re-entrant: akerr_init() reserves the library's own status band and names its + * own codes, and every one of those calls goes through a public entry point + * that calls akerr_init() again. The guard against that recursion has to be + * per-thread, or a second thread arriving mid-initialization would see the flag + * the first thread raised on its way in and walk tables that are still being + * built. + * + * Nothing in main() touches the library before the threads start, so the race + * is real: whichever thread wins does the initializing, and the rest must block + * until it is finished rather than proceed on half-built tables. + * + * What proves it ran once rather than several times: each thread reserves its + * own status range as its first act. A second pass through akerr_init() would + * memset the range table, so a reservation made by a thread that raced ahead + * would silently vanish -- exactly the failure the pre-1.0.0 library had. After + * the join, every thread's reservation must still be there, still attributed to + * that thread. + */ + +static int thread_range_base(int id) +{ + return 400000 + (id * 16); +} + +static void *init_racer(void *raw) +{ + akerr_ThreadArg *arg = raw; + akerr_ErrorContext *e; + char owner[32]; + char name[32]; + int base = thread_range_base(arg->id); + + snprintf(owner, sizeof(owner), "init-thread-%d", arg->id); + snprintf(name, sizeof(name), "Thread %d Error", arg->id); + pthread_barrier_wait(arg->barrier); + + /* First touch of the library from this thread, and for one of them the + * first touch in the process. */ + e = akerr_reserve_status_range(base, 16, owner); + AKERR_TCHECK(arg, e == NULL); + RELEASE_ERROR(e); + + e = akerr_register_status_name(owner, base, name); + AKERR_TCHECK(arg, e == NULL); + RELEASE_ERROR(e); + + /* The library's own band was reserved by whichever thread initialized, and + * every thread must see it as taken -- including the one that did it. A + * second initialization would have wiped the reservation and let this + * through. */ + e = akerr_reserve_status_range(0, AKERR_RESERVED_STATUS_COUNT, owner); + AKERR_TCHECK(arg, e != NULL); + if ( e != NULL ) { + AKERR_TCHECK(arg, e->status == AKERR_STATUS_RANGE_OVERLAP); + AKERR_TCHECK(arg, strstr(e->message, AKERR_LIBRARY_OWNER) != NULL); + } + RELEASE_ERROR(e); + + /* The name tables are complete as seen from every thread: the library's own + * codes, the generated errno names, and this thread's own registration. */ + AKERR_TCHECK(arg, strcmp(akerr_name_for_status(AKERR_VALUE, NULL), + "Value Error") == 0); + AKERR_TCHECK(arg, strcmp(akerr_name_for_status(AKERR_NULLPOINTER, NULL), + "Null Pointer Error") == 0); + AKERR_TCHECK(arg, strcmp(akerr_name_for_status(EACCES, NULL), + "Unknown Error") != 0); + AKERR_TCHECK(arg, strcmp(akerr_name_for_status(base, NULL), name) == 0); + + /* And an error raised from this thread renders with a name, which is the + * whole point of the tables being complete. */ + PREPARE_ERROR(errctx); + ATTEMPT { + FAIL_BREAK(errctx, AKERR_TYPE, "raised during init race by thread %d", + arg->id); + } CLEANUP { + } PROCESS(errctx) { + } HANDLE(errctx, AKERR_TYPE) { + AKERR_TCHECK(arg, strstr(errctx->stacktracebuf, "Type Error") != NULL); + } FINISH_NORETURN(errctx); + + return NULL; +} + +int main(void) +{ + /* Installed before anything initializes: akerr_init() only supplies the + * default logger when this is still NULL. */ + akerr_log_method = &akerr_thread_logger; + + int failures = akerr_run_threads(&init_racer); + AKERR_CHECK(failures == 0); + + /* Every thread's reservation survived the race, under its own owner. */ + for ( int id = 1; id <= AKERR_TEST_THREADS; id++ ) { + char owner[32]; + char name[32]; + int base = thread_range_base(id); + + snprintf(owner, sizeof(owner), "init-thread-%d", id); + snprintf(name, sizeof(name), "Thread %d Error", id); + AKERR_CHECK(strcmp(akerr_name_for_status(base, NULL), name) == 0); + AKERR_CHECK_RAISES(akerr_reserve_status_range(base, 16, "verifier"), + AKERR_STATUS_RANGE_OVERLAP); + AKERR_CHECK_MESSAGE_CONTAINS(owner); + } + + /* Nothing leaked a pool slot on the way through. */ + AKERR_CHECK(akerr_slots_in_use() == 0); + + fprintf(stderr, "err_threads_init ok (%d threads raced initialization)\n", + AKERR_TEST_THREADS); + return 0; +} diff --git a/tests/err_threads_pool.c b/tests/err_threads_pool.c new file mode 100644 index 0000000..54a33e4 --- /dev/null +++ b/tests/err_threads_pool.c @@ -0,0 +1,138 @@ +#include "akerror.h" +#include "err_capture.h" +#include "err_threads.h" +#include + +/* + * The error pool under contention. + * + * AKERR_ARRAY_ERROR is a fixed 128-slot array shared by every thread in the + * process, and a slot is checked out by finding refcount == 0 and taking a + * reference. Those two steps have to be one operation: a scan that returned an + * unclaimed slot would hand the same one to every thread that scanned before + * the first of them incremented the count, and each would then format its own + * error into the same buffers. That failure is invisible to a single-threaded + * test and produces a garbled message rather than a crash, so this test asserts + * exclusivity directly. + * + * akerr_slot_owner[] (see err_threads.h) is the test's own record of who holds + * which slot, kept with atomics. Every check-out claims its slot and every + * release drops the claim; a slot handed to two threads at once is caught by + * the claim failing, whether or not the resulting message is garbled. + * + * Each thread also asserts that the error it raised is the error it handles, + * message and all. That is the same property from the other end: a context + * cannot be exclusively ours if another thread's text shows up in it. + */ + +#define ITERATIONS 2000 + +/* Raise an error and take ownership of whatever slot it came from. */ +static akerr_ErrorContext AKERR_NOIGNORE *boom(akerr_ThreadArg *arg) +{ + PREPARE_ERROR(e); + + FAIL(e, AKERR_VALUE, "raised by thread %d", arg->id); + AKERR_TCHECK(arg, akerr_slot_claim(e->arrayid, arg->id) == 0); + return e; +} + +static akerr_ErrorContext AKERR_NOIGNORE *ignorable(akerr_ThreadArg *arg) +{ + PREPARE_ERROR(e); + + FAIL_RETURN(e, AKERR_IO, "ignored by thread %d", arg->id); +} + +/* One raise -> catch -> handle cycle, exclusively owned from end to end. */ +static void one_cycle(akerr_ThreadArg *arg) +{ + char expected[64]; + PREPARE_ERROR(e); + + snprintf(expected, sizeof(expected), "raised by thread %d", arg->id); + ATTEMPT { + CATCH(e, boom(arg)); + } CLEANUP { + } PROCESS(e) { + /* case 0: the error we just raised came back clean, which can only + * mean another thread wrote over this context. */ + int error_was_lost = 1; + AKERR_TCHECK(arg, error_was_lost == 0); + } HANDLE(e, AKERR_VALUE) { + AKERR_TCHECK(arg, akerr_slot_holder(e->arrayid) == arg->id); + AKERR_TCHECK(arg, strcmp(e->message, expected) == 0); + AKERR_TCHECK(arg, strstr(e->stacktracebuf, expected) != NULL); + /* Give the slot up before FINISH releases the context: the other order + * hands it back to the pool while this thread still claims it. */ + akerr_slot_drop(e->arrayid); + } FINISH_NORETURN(e); +} + +/* The same property against the raw pool API, with no macros in between. */ +static void one_checkout(akerr_ThreadArg *arg) +{ + akerr_ErrorContext *e = akerr_next_error(); + + AKERR_TCHECK(arg, e != NULL); + if ( e == NULL ) { + return; + } + /* The context arrives already holding its reference. */ + AKERR_TCHECK(arg, e->refcount == 1); + AKERR_TCHECK(arg, akerr_slot_claim(e->arrayid, arg->id) == 0); + AKERR_TCHECK(arg, akerr_slot_holder(e->arrayid) == arg->id); + akerr_slot_drop(e->arrayid); + RELEASE_ERROR(e); + AKERR_TCHECK(arg, e == NULL); +} + +static void *pool_body(void *raw) +{ + akerr_ThreadArg *arg = raw; + char expected[64]; + + snprintf(expected, sizeof(expected), "ignored by thread %d", arg->id); + pthread_barrier_wait(arg->barrier); + + for ( int i = 0; i < ITERATIONS; i++ ) { + one_cycle(arg); + one_checkout(arg); + } + + /* An ignored error is a fact about the thread that ignored it: each thread + * must see its own, not the last one any thread swallowed. */ + IGNORE(ignorable(arg)); + AKERR_TCHECK(arg, __akerr_last_ignored != NULL); + if ( __akerr_last_ignored != NULL ) { + AKERR_TCHECK(arg, __akerr_last_ignored->status == AKERR_IO); + AKERR_TCHECK(arg, strcmp(__akerr_last_ignored->message, expected) == 0); + } + /* IGNORE keeps the reference by design; hand it back so the pool is empty + * at the end of the test. */ + RELEASE_ERROR(__akerr_last_ignored); + + return NULL; +} + +int main(void) +{ + akerr_log_method = &akerr_thread_logger; + akerr_init(); + AKERR_CHECK(akerr_slots_in_use() == 0); + + int failures = akerr_run_threads(&pool_body); + AKERR_CHECK(failures == 0); + + /* Every context went back to the pool, and every claim was dropped. */ + AKERR_CHECK(akerr_slots_in_use() == 0); + for ( int i = 0; i < AKERR_MAX_ARRAY_ERROR; i++ ) { + AKERR_CHECK(akerr_slot_holder(i) == 0); + } + /* Each thread's IGNORE reported through the log method. */ + AKERR_CHECK(akerr_thread_logs() >= AKERR_TEST_THREADS); + + fprintf(stderr, "err_threads_pool ok (%d threads x %d cycles)\n", + AKERR_TEST_THREADS, ITERATIONS); + return 0; +} diff --git a/tests/err_threads_registry.c b/tests/err_threads_registry.c new file mode 100644 index 0000000..e7ffc01 --- /dev/null +++ b/tests/err_threads_registry.c @@ -0,0 +1,137 @@ +#include "akerror.h" +#include "err_capture.h" +#include "err_threads.h" +#include + +/* + * The status registry under contention. + * + * Two properties, and they fail differently: + * + * - A reservation is a decision, not a write. The overlap scan and the entry + * that follows it have to be one operation, or two threads claiming the + * same range both find the table clear and both believe they own it. That + * is silent: neither gets an error, and the collision surfaces much later + * as one component's status rendering under another's name. The contested + * range below is claimed by every thread at once and exactly one may win. + * + * - The name table is an open-addressed hash table with linear probing. A + * concurrent insert that another thread's probe walks through -- an entry + * half claimed, a count incremented before the slot was marked used -- loses + * names or writes outside the table. So every thread registers a block of + * names and reads each one back while the others are still writing, and + * interleaves lookups of a name nobody is touching. + * + * Ownership enforcement has to hold under contention too: after every thread + * has reserved, each one tries to name a status inside its neighbour's range + * and must be refused. The barrier before that is what makes the expected + * refusal exactly AKERR_STATUS_NAME_FOREIGN rather than sometimes + * AKERR_STATUS_NAME_UNRESERVED, which is a real distinction and not just test + * tidiness: FOREIGN means the registry knew who owned it. + */ + +#define NAMES_PER_THREAD 64 +#define CONTESTED_FIRST 900000 +#define CONTESTED_COUNT 64 + +static int contested_winners; + +static int thread_range_base(int id) +{ + return 500000 + (id * 1000); +} + +static void *registry_body(void *raw) +{ + akerr_ThreadArg *arg = raw; + akerr_ErrorContext *e; + char owner[32]; + int base = thread_range_base(arg->id); + int victim = thread_range_base((arg->id % AKERR_TEST_THREADS) + 1); + + snprintf(owner, sizeof(owner), "registry-%d", arg->id); + pthread_barrier_wait(arg->barrier); + + /* One range, every thread, distinct owners. Exactly one may come back + * successful; the rest must be told who won. */ + e = akerr_reserve_status_range(CONTESTED_FIRST, CONTESTED_COUNT, owner); + if ( e == NULL ) { + __atomic_fetch_add(&contested_winners, 1, __ATOMIC_RELAXED); + } else { + AKERR_TCHECK(arg, e->status == AKERR_STATUS_RANGE_OVERLAP); + AKERR_TCHECK(arg, strstr(e->message, "registry-") != NULL); + RELEASE_ERROR(e); + } + + /* This thread's own range, which nobody contests. */ + e = akerr_reserve_status_range(base, NAMES_PER_THREAD, owner); + AKERR_TCHECK(arg, e == NULL); + RELEASE_ERROR(e); + + for ( int i = 0; i < NAMES_PER_THREAD; i++ ) { + char name[48]; + + snprintf(name, sizeof(name), "registry-%d name %d", arg->id, i); + e = akerr_register_status_name(owner, base + i, name); + AKERR_TCHECK(arg, e == NULL); + RELEASE_ERROR(e); + /* Read it back while the other threads are still inserting. */ + AKERR_TCHECK(arg, strcmp(akerr_name_for_status(base + i, NULL), name) == 0); + /* And an entry nobody is touching: a probe sequence that a concurrent + * insert walked off loses names that were already there. */ + AKERR_TCHECK(arg, strcmp(akerr_name_for_status(AKERR_VALUE, NULL), + "Value Error") == 0); + } + + /* Everyone has reserved by the time anyone tries to trespass. */ + pthread_barrier_wait(arg->barrier); + + e = akerr_register_status_name(owner, victim, "Hijack"); + AKERR_TCHECK(arg, e != NULL); + if ( e != NULL ) { + AKERR_TCHECK(arg, e->status == AKERR_STATUS_NAME_FOREIGN); + } + RELEASE_ERROR(e); + + return NULL; +} + +int main(void) +{ + akerr_log_method = &akerr_thread_logger; + akerr_init(); + AKERR_CHECK(akerr_slots_in_use() == 0); + + int failures = akerr_run_threads(®istry_body); + AKERR_CHECK(failures == 0); + + /* The contested range went to exactly one owner. */ + AKERR_CHECK(contested_winners == 1); + + /* Every name every thread registered is present and correct: nothing was + * dropped, overwritten, or attributed to the wrong thread. */ + for ( int id = 1; id <= AKERR_TEST_THREADS; id++ ) { + int base = thread_range_base(id); + + for ( int i = 0; i < NAMES_PER_THREAD; i++ ) { + char expected[48]; + + snprintf(expected, sizeof(expected), "registry-%d name %d", id, i); + AKERR_CHECK(strcmp(akerr_name_for_status(base + i, NULL), expected) == 0); + } + /* The trespass attempt did not leave a name behind. */ + AKERR_CHECK(strcmp(akerr_name_for_status(base, NULL), "Hijack") != 0); + } + + /* The library's own entries survived every one of those inserts. */ + AKERR_CHECK(strcmp(akerr_name_for_status(AKERR_VALUE, NULL), "Value Error") == 0); + AKERR_CHECK(strcmp(akerr_name_for_status(AKERR_STATUS_NAME_FOREIGN, NULL), + "Foreign Status Name") == 0); + + /* Thousands of refusals and registrations later, the pool is empty. */ + AKERR_CHECK(akerr_slots_in_use() == 0); + + fprintf(stderr, "err_threads_registry ok (%d threads x %d names)\n", + AKERR_TEST_THREADS, NAMES_PER_THREAD); + return 0; +}