diff --git a/CMakeLists.txt b/CMakeLists.txt index f27b895..c501683 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,7 +1,7 @@ cmake_minimum_required(VERSION 3.10) # 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 +# 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 diff --git a/README.md b/README.md index 9af9e16..60d8748 100644 --- a/README.md +++ b/README.md @@ -170,7 +170,7 @@ the exit code was the status truncated to a byte, and every consumer status starts at 256. Use `akerr_exit()` instead of `exit()` — see [docs/exit-status.md](docs/exit-status.md). No ABI break. -2.0.0 makes the library thread safe. That is an ABI break — `__akerr_last_ignored` +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. diff --git a/UPGRADING.md b/UPGRADING.md index ec9a877..7c858ff 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -52,7 +52,7 @@ accident. What moved at the ABI: -* `__akerr_last_ignored` is thread-local storage. An ignored error is a fact +* `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 diff --git a/docs/thread-safety.md b/docs/thread-safety.md index 76ccc36..2769839 100644 --- a/docs/thread-safety.md +++ b/docs/thread-safety.md @@ -14,9 +14,12 @@ What that covers: 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. +* **Per-thread state.** `IGNORE` copies the swallowed context into its + thread-local `akerr_last_ignored` snapshot before releasing the pool slot. + The snapshot remains valid until that thread ignores another error, so a + later pool checkout cannot overwrite it. The snapshot and the last-ditch + context used to report `akerr_release_error(NULL)` are thread-local, so + concurrent calls cannot overwrite each other's state. * **Handing a context from one thread to another.** A context is not thread state — it lives in `AKERR_ARRAY_ERROR`, which is process-global — so it outlives the thread that raised it. The reference count is the only field the diff --git a/include/akerror.tmpl.h b/include/akerror.tmpl.h index ede8fcc..e495a65 100644 --- a/include/akerror.tmpl.h +++ b/include/akerror.tmpl.h @@ -15,7 +15,7 @@ * 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 + * 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. * @@ -173,11 +173,12 @@ extern akerr_ErrorContext AKERR_ARRAY_ERROR[AKERR_MAX_ARRAY_ERROR]; extern akerr_ErrorUnhandledErrorHandler akerr_handler_unhandled_error; extern akerr_ErrorLogFunction akerr_log_method; /* - * 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. + * IGNORE()'s public per-thread snapshot. IGNORE() copies the swallowed error here + * before releasing its pool context, so this remains a useful debugging aid + * after the pool slot is reused. The snapshot is read-only and is replaced by + * the next ignored error. Thread local only when AKERR_THREAD_SAFE is 1. */ -extern AKERR_THREAD_LOCAL akerr_ErrorContext *__akerr_last_ignored; +static AKERR_THREAD_LOCAL akerr_ErrorContext akerr_last_ignored; /* * Drop one reference, returning NULL once the last one is gone so the caller can @@ -434,10 +435,20 @@ akerr_ErrorContext AKERR_NOIGNORE *__akerr_copy_string(char *destination, int ca FINISH_LOGIC(__err_context, true); #define IGNORE(__stmt) \ - __akerr_last_ignored = __stmt; \ - if ( __akerr_last_ignored != NULL ) { \ - LOG_ERROR_WITH_MESSAGE(__akerr_last_ignored, "** IGNORED ERROR **"); \ - } + do { \ + akerr_ErrorContext *__akerr_ignored = __stmt; \ + if ( __akerr_ignored != NULL ) { \ + memcpy(&akerr_last_ignored, __akerr_ignored, \ + sizeof(akerr_last_ignored)); \ + akerr_last_ignored.stacktracebufptr = \ + (char *)&akerr_last_ignored.stacktracebuf; \ + akerr_ErrorContext *__akerr_ignored_snapshot = \ + &akerr_last_ignored; \ + LOG_ERROR_WITH_MESSAGE(__akerr_ignored_snapshot, \ + "** IGNORED ERROR **"); \ + RELEASE_ERROR(__akerr_ignored); \ + } \ + } while ( 0 ) #define CLEANUP \ }; diff --git a/src/error.c b/src/error.c index 66b3e66..e73177c 100644 --- a/src/error.c +++ b/src/error.c @@ -20,10 +20,10 @@ * 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. + * can be writing at once. The per-thread IGNORE() snapshot lives in the public + * template header because the macro copies into it at the call site. */ 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; @@ -236,7 +236,6 @@ static void akerr_init_state(void) 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; diff --git a/tests/MUTATION.md b/tests/MUTATION.md index 5362d74..a5cfcd7 100644 --- a/tests/MUTATION.md +++ b/tests/MUTATION.md @@ -107,7 +107,7 @@ The remaining survivors are dominated by: * **Equivalent mutants** in `akerr_init`: deleting the `memset`/`NULL` setup of file-scope statics (`AKERR_ARRAY_ERROR`, `__akerr_last_ditch`, - `__akerr_last_ignored`) changes nothing, because C already zero-initializes + `akerr_last_ignored`) changes nothing, because C already zero-initializes objects with static storage duration. `int oldid = 0;` → `1` is likewise dead: it is overwritten before use, and so is clearing `akerr_initializing` at the end of initialization — nothing reads that flag once the once-routine diff --git a/tests/err_ignore.c b/tests/err_ignore.c index f52ca55..0c86629 100644 --- a/tests/err_ignore.c +++ b/tests/err_ignore.c @@ -1,11 +1,8 @@ #include "akerror.h" #include "err_capture.h" +#include -/* - * IGNORE deliberately swallows an error: it records the context in - * __akerr_last_ignored, logs it with an "IGNORED ERROR" marker, and lets - * execution continue. - */ +/* IGNORE snapshots and logs an error, releases its pool slot, then continues. */ akerr_ErrorContext *boom(void) { @@ -21,11 +18,19 @@ int main(void) PREPARE_ERROR(e); (void)e; - IGNORE(boom()); + /* More failures than the pool has slots must remain safe: a leaking + * IGNORE used to exhaust the pool and terminate the process here. The + * copied snapshot must also survive the slot being reused on the next + * iteration. */ + for ( int i = 0; i < AKERR_MAX_ARRAY_ERROR + 1; i++ ) { + IGNORE(boom()); + AKERR_CHECK(akerr_last_ignored.status == AKERR_VALUE); + AKERR_CHECK(strcmp(akerr_last_ignored.message, + "this error is ignored on purpose") == 0); + AKERR_CHECK(akerr_slots_in_use() == 0); + } reached_after_ignore = 1; - AKERR_CHECK(__akerr_last_ignored != NULL); - AKERR_CHECK(__akerr_last_ignored->status == AKERR_VALUE); AKERR_CHECK(reached_after_ignore == 1); AKERR_CHECK_CONTAINS("IGNORED ERROR"); AKERR_CHECK_CONTAINS("this error is ignored on purpose"); diff --git a/tests/err_threads_pool.c b/tests/err_threads_pool.c index 54a33e4..8f38f07 100644 --- a/tests/err_threads_pool.c +++ b/tests/err_threads_pool.c @@ -100,17 +100,21 @@ static void *pool_body(void *raw) 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's snapshot is thread-local while logging and remains valid after + * release. Concurrent ignored errors must all return their pool slots. */ 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); + AKERR_TCHECK(arg, akerr_last_ignored.status == AKERR_IO); + AKERR_TCHECK(arg, strcmp(akerr_last_ignored.message, expected) == 0); + + /* Reuse a slot after IGNORE and prove that the copied snapshot did not + * become an alias for the newly acquired context. */ + akerr_ErrorContext *reused = akerr_next_error(); + AKERR_TCHECK(arg, reused != NULL); + if ( reused != NULL ) { + RELEASE_ERROR(reused); } - /* 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); + AKERR_TCHECK(arg, akerr_last_ignored.status == AKERR_IO); + AKERR_TCHECK(arg, strcmp(akerr_last_ignored.message, expected) == 0); return NULL; }