Avoid wiping full error context on release
This commit is contained in:
@@ -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.
|
||||
|
||||
12
src/error.c
12
src/error.c
@@ -380,10 +380,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)
|
||||
{
|
||||
@@ -401,7 +401,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;
|
||||
|
||||
@@ -1,24 +1,32 @@
|
||||
#include "akerror.h"
|
||||
#include "err_capture.h"
|
||||
#include <string.h>
|
||||
|
||||
/*
|
||||
* 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;
|
||||
|
||||
Reference in New Issue
Block a user