diff --git a/docs/thread-safety.md b/docs/thread-safety.md index 62ecd08..80ca0a1 100644 --- a/docs/thread-safety.md +++ b/docs/thread-safety.md @@ -14,11 +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.** `IGNORE` uses `__akerr_last_ignored` as a scratch pointer - while it logs an error, then releases the context and clears the pointer. - That scratch pointer and the last-ditch context used to report - `akerr_release_error(NULL)` are thread-local, so concurrent calls cannot - overwrite each other's state. +* **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 ed5e5e5..5d46039 100644 --- a/include/akerror.tmpl.h +++ b/include/akerror.tmpl.h @@ -173,12 +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; /* - * IGNORE()'s per-thread scratch pointer. It is non-NULL only while IGNORE() - * logs the swallowed error; IGNORE() releases the context and clears this - * pointer before returning to its caller. Thread local only when - * AKERR_THREAD_SAFE is 1. + * IGNORE()'s 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 @@ -435,11 +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 **"); \ - RELEASE_ERROR(__akerr_last_ignored); \ - } + 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 4182d50..1c49874 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; @@ -233,7 +233,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/err_ignore.c b/tests/err_ignore.c index 6282928..3559fae 100644 --- a/tests/err_ignore.c +++ b/tests/err_ignore.c @@ -1,7 +1,8 @@ #include "akerror.h" #include "err_capture.h" +#include -/* IGNORE logs and releases an error, then lets execution continue. */ +/* IGNORE snapshots and logs an error, releases its pool slot, then continues. */ akerr_ErrorContext *boom(void) { @@ -18,10 +19,14 @@ int main(void) (void)e; /* More failures than the pool has slots must remain safe: a leaking - * IGNORE used to exhaust the pool and terminate the process here. */ + * 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 == NULL); + 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; diff --git a/tests/err_threads_pool.c b/tests/err_threads_pool.c index 1b80908..530fa51 100644 --- a/tests/err_threads_pool.c +++ b/tests/err_threads_pool.c @@ -90,6 +90,9 @@ static void one_checkout(akerr_ThreadArg *arg) 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++ ) { @@ -97,10 +100,21 @@ static void *pool_body(void *raw) one_checkout(arg); } - /* IGNORE's scratch pointer is thread-local while logging and cleared after + /* 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); + 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); + } + AKERR_TCHECK(arg, __akerr_last_ignored.status == AKERR_IO); + AKERR_TCHECK(arg, strcmp(__akerr_last_ignored.message, expected) == 0); return NULL; }