Release ignored error contexts #21
@@ -14,9 +14,11 @@ What that covers:
|
|||||||
against each other and against lookups. Two threads reserving the same range
|
against each other and against lookups. Two threads reserving the same range
|
||||||
cannot both win — exactly one gets `NULL` and the other gets
|
cannot both win — exactly one gets `NULL` and the other gets
|
||||||
`AKERR_STATUS_RANGE_OVERLAP` naming the winner.
|
`AKERR_STATUS_RANGE_OVERLAP` naming the winner.
|
||||||
* **Per-thread state.** The context behind `IGNORE` (`__akerr_last_ignored`) and
|
* **Per-thread state.** `IGNORE` uses `__akerr_last_ignored` as a scratch pointer
|
||||||
the last-ditch context used to report `akerr_release_error(NULL)` are
|
while it logs an error, then releases the context and clears the pointer.
|
||||||
thread-local, so one thread's ignored error is never another's.
|
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
|
* **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
|
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
|
outlives the thread that raised it. The reference count is the only field the
|
||||||
|
|||||||
@@ -173,9 +173,10 @@ extern akerr_ErrorContext AKERR_ARRAY_ERROR[AKERR_MAX_ARRAY_ERROR];
|
|||||||
extern akerr_ErrorUnhandledErrorHandler akerr_handler_unhandled_error;
|
extern akerr_ErrorUnhandledErrorHandler akerr_handler_unhandled_error;
|
||||||
extern akerr_ErrorLogFunction akerr_log_method;
|
extern akerr_ErrorLogFunction akerr_log_method;
|
||||||
/*
|
/*
|
||||||
* The error IGNORE() last swallowed, per thread: an ignored error is a fact
|
* IGNORE()'s per-thread scratch pointer. It is non-NULL only while IGNORE()
|
||||||
* about the thread that ignored it, and one shared slot would have two threads
|
* logs the swallowed error; IGNORE() releases the context and clears this
|
||||||
* overwriting each other's. Thread local only when AKERR_THREAD_SAFE is 1.
|
* pointer before returning to its caller. Thread local only when
|
||||||
|
* AKERR_THREAD_SAFE is 1.
|
||||||
*/
|
*/
|
||||||
extern AKERR_THREAD_LOCAL akerr_ErrorContext *__akerr_last_ignored;
|
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; \
|
__akerr_last_ignored = __stmt; \
|
||||||
if ( __akerr_last_ignored != NULL ) { \
|
if ( __akerr_last_ignored != NULL ) { \
|
||||||
LOG_ERROR_WITH_MESSAGE(__akerr_last_ignored, "** IGNORED ERROR **"); \
|
LOG_ERROR_WITH_MESSAGE(__akerr_last_ignored, "** IGNORED ERROR **"); \
|
||||||
|
RELEASE_ERROR(__akerr_last_ignored); \
|
||||||
|
logikoma marked this conversation as resolved
Outdated
|
|||||||
}
|
}
|
||||||
|
|
||||||
#define CLEANUP \
|
#define CLEANUP \
|
||||||
|
|||||||
@@ -1,11 +1,7 @@
|
|||||||
#include "akerror.h"
|
#include "akerror.h"
|
||||||
#include "err_capture.h"
|
#include "err_capture.h"
|
||||||
|
|
||||||
/*
|
/* IGNORE logs and releases an error, then lets execution continue. */
|
||||||
* IGNORE deliberately swallows an error: it records the context in
|
|
||||||
* __akerr_last_ignored, logs it with an "IGNORED ERROR" marker, and lets
|
|
||||||
* execution continue.
|
|
||||||
*/
|
|
||||||
|
|
||||||
akerr_ErrorContext *boom(void)
|
akerr_ErrorContext *boom(void)
|
||||||
{
|
{
|
||||||
@@ -21,11 +17,15 @@ int main(void)
|
|||||||
PREPARE_ERROR(e);
|
PREPARE_ERROR(e);
|
||||||
(void)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;
|
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(reached_after_ignore == 1);
|
||||||
AKERR_CHECK_CONTAINS("IGNORED ERROR");
|
AKERR_CHECK_CONTAINS("IGNORED ERROR");
|
||||||
AKERR_CHECK_CONTAINS("this error is ignored on purpose");
|
AKERR_CHECK_CONTAINS("this error is ignored on purpose");
|
||||||
|
|||||||
@@ -90,9 +90,6 @@ static void one_checkout(akerr_ThreadArg *arg)
|
|||||||
static void *pool_body(void *raw)
|
static void *pool_body(void *raw)
|
||||||
{
|
{
|
||||||
akerr_ThreadArg *arg = raw;
|
akerr_ThreadArg *arg = raw;
|
||||||
char expected[64];
|
|
||||||
|
|
||||||
snprintf(expected, sizeof(expected), "ignored by thread %d", arg->id);
|
|
||||||
pthread_barrier_wait(arg->barrier);
|
pthread_barrier_wait(arg->barrier);
|
||||||
|
|
||||||
for ( int i = 0; i < ITERATIONS; i++ ) {
|
for ( int i = 0; i < ITERATIONS; i++ ) {
|
||||||
@@ -100,17 +97,10 @@ static void *pool_body(void *raw)
|
|||||||
one_checkout(arg);
|
one_checkout(arg);
|
||||||
}
|
}
|
||||||
|
|
||||||
/* An ignored error is a fact about the thread that ignored it: each thread
|
/* IGNORE's scratch pointer is thread-local while logging and cleared after
|
||||||
* must see its own, not the last one any thread swallowed. */
|
* release. Concurrent ignored errors must all return their pool slots. */
|
||||||
IGNORE(ignorable(arg));
|
IGNORE(ignorable(arg));
|
||||||
AKERR_TCHECK(arg, __akerr_last_ignored != NULL);
|
AKERR_TCHECK(arg, __akerr_last_ignored == NULL);
|
||||||
|
andrew marked this conversation as resolved
Outdated
andrew
commented
IGNORE() retained a reference to the last ignored error on purpose, so that subsequent errors that may be related could reference it. Instead of completely abandoning it (and the pattern that allows it), why not turn __akerr_last_ignored into a static variable, IGNORE() does a IGNORE() retained a reference to the last ignored error on purpose, so that subsequent errors that may be related could reference it. Instead of completely abandoning it (and the pattern that allows it), why not turn __akerr_last_ignored into a static variable, IGNORE() does a `memcpy()` into it from the exception being ignored, then releases the exception? That seems like it would give us the best of both worlds.
andrew
commented
@logikoma ^ implement this > IGNORE() retained a reference to the last ignored error on purpose, so that subsequent errors that may be related could reference it. Instead of completely abandoning it (and the pattern that allows it), why not turn __akerr_last_ignored into a static variable, IGNORE() does a `memcpy()` into it from the exception being ignored, then releases the exception? That seems like it would give us the best of both worlds.
@logikoma ^ implement this
|
|||||||
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);
|
|
||||||
|
|
||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
|
|||||||
@logikoma the
__akerr_last_ignoredvariable is only really useful to users of the public facing API, so I'm not sure it makes sense to prefix it with__as part of the private API. Let's remove the__prefix on this variable and make it public.