diff --git a/docs/thread-safety.md b/docs/thread-safety.md index 76ccc36..62ecd08 100644 --- a/docs/thread-safety.md +++ b/docs/thread-safety.md @@ -14,9 +14,11 @@ 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` 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. * **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..ed5e5e5 100644 --- a/include/akerror.tmpl.h +++ b/include/akerror.tmpl.h @@ -173,9 +173,10 @@ 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 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. */ extern AKERR_THREAD_LOCAL akerr_ErrorContext *__akerr_last_ignored; @@ -437,6 +438,7 @@ akerr_ErrorContext AKERR_NOIGNORE *__akerr_copy_string(char *destination, int ca __akerr_last_ignored = __stmt; \ if ( __akerr_last_ignored != NULL ) { \ LOG_ERROR_WITH_MESSAGE(__akerr_last_ignored, "** IGNORED ERROR **"); \ + RELEASE_ERROR(__akerr_last_ignored); \ } #define CLEANUP \ diff --git a/tests/err_ignore.c b/tests/err_ignore.c index f52ca55..6282928 100644 --- a/tests/err_ignore.c +++ b/tests/err_ignore.c @@ -1,11 +1,7 @@ #include "akerror.h" #include "err_capture.h" -/* - * 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 logs and releases an error, then lets execution continue. */ akerr_ErrorContext *boom(void) { @@ -21,11 +17,15 @@ 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. */ + for ( int i = 0; i < AKERR_MAX_ARRAY_ERROR + 1; i++ ) { + IGNORE(boom()); + AKERR_CHECK(__akerr_last_ignored == NULL); + 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..1b80908 100644 --- a/tests/err_threads_pool.c +++ b/tests/err_threads_pool.c @@ -90,9 +90,6 @@ 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++ ) { @@ -100,17 +97,10 @@ 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 scratch pointer is thread-local while logging and cleared 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); - } - /* 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 == NULL); return NULL; }