diff --git a/UPGRADING.md b/UPGRADING.md index 7c858ff..ee14e1c 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -132,6 +132,11 @@ One recursive lock covers both the pool and the registry, so error correctness there is worth more than throughput, but a program that raises errors in a hot loop will feel it. +Releasing the last reference to a context remains serialized under that same +pool lock, but it now resets only the handled/status/reported state, the string +heads, and the stack-trace cursor. It no longer wipes the whole context buffer, +so release is a fixed handful of stores rather than a tens-of-kilobytes write. + 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. diff --git a/src/error.c b/src/error.c index e73177c..0f403dc 100644 --- a/src/error.c +++ b/src/error.c @@ -379,10 +379,10 @@ akerr_ErrorContext *akerr_next_error() } /* - * The wipe returns the slot to the pool, so it and the decrement that triggers + * The reset 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. + * its error into it while the releasing thread was still resetting it. */ akerr_ErrorContext *akerr_release_error(akerr_ErrorContext *err) { @@ -400,7 +400,13 @@ akerr_ErrorContext *akerr_release_error(akerr_ErrorContext *err) } if ( err->refcount == 0 ) { oldid = err->arrayid; - memset(err, 0x00, sizeof(akerr_ErrorContext)); + err->handled = false; + err->status = 0; + err->reported = false; + err->message[0] = '\0'; + err->fname[0] = '\0'; + err->function[0] = '\0'; + err->stacktracebuf[0] = '\0'; err->stacktracebufptr = (char *)&err->stacktracebuf; err->arrayid = oldid; remaining = NULL; diff --git a/tests/err_release_clears.c b/tests/err_release_clears.c index 126eb6b..0e1dd23 100644 --- a/tests/err_release_clears.c +++ b/tests/err_release_clears.c @@ -1,24 +1,32 @@ #include "akerror.h" #include "err_capture.h" -#include /* - * Releasing an error context back to the pool must wipe it, so the next caller - * that checks it out never sees stale status/message/stacktrace from a previous - * error. Mutation testing showed the clearing memset in akerr_release_error - * could be deleted without any test noticing. + * Releasing an error context back to the pool must reset the state that affects + * the next caller. In particular, a handled error must not make a fresh error + * look handled when its slot is recycled. */ +static int unhandled_calls = 0; +static int unhandled_status = 0; + +static void test_unhandled_handler(akerr_ErrorContext *errctx) +{ + unhandled_calls++; + unhandled_status = (errctx != NULL) ? errctx->status : 0; +} + akerr_ErrorContext *boom(void) { PREPARE_ERROR(e); - FAIL_RETURN(e, AKERR_VALUE, "stale dirty message that must not survive"); + FAIL_RETURN(e, AKERR_VALUE, "first error is handled"); } int main(void) { akerr_capture_install(); akerr_init(); + akerr_handler_unhandled_error = &test_unhandled_handler; /* Raise and fully handle an error; FINISH_NORETURN releases it to the pool. */ PREPARE_ERROR(e); @@ -32,13 +40,31 @@ int main(void) AKERR_CHECK(e == NULL); - /* The next context handed out is the slot we just released: it must be clean. */ + /* A fresh error in the recycled slot must not inherit handled=true. */ + PREPARE_ERROR(fresh); + ATTEMPT { + CATCH(fresh, boom()); + } CLEANUP { + } PROCESS(fresh) { + } FINISH_NORETURN(fresh); + + AKERR_CHECK(unhandled_calls == 1); + AKERR_CHECK(unhandled_status == AKERR_VALUE); + AKERR_CHECK(fresh == NULL); + + /* The next context handed out is the same slot, with recycle state reset. */ akerr_ErrorContext *slot = akerr_next_error(); AKERR_CHECK(slot != NULL); + AKERR_CHECK(slot->handled == false); AKERR_CHECK(slot->status == 0); + AKERR_CHECK(slot->reported == false); AKERR_CHECK(slot->message[0] == '\0'); + AKERR_CHECK(slot->fname[0] == '\0'); + AKERR_CHECK(slot->function[0] == '\0'); AKERR_CHECK(slot->stacktracebuf[0] == '\0'); - AKERR_CHECK(strstr(slot->message, "stale dirty message") == NULL); + AKERR_CHECK(slot->stacktracebufptr == (char *)&slot->stacktracebuf); + RELEASE_ERROR(slot); + AKERR_CHECK(akerr_slots_in_use() == 0); fprintf(stderr, "err_release_clears ok\n"); return 0;