Files

104 lines
5.2 KiB
Markdown
Raw Permalink Normal View History

# Record
2026-07-30 13:22:50 -04:00
**Outstanding work is in the issue tracker, not in this file:**
<https://source.starfort.tech/andrew/libakerror/issues>
Issues are labelled by kind and blast radius, and milestoned by what they can land
in: `2.0.x` for anything that breaks no ABI, `2.1.0` for additive surface, `3.0.0`
for the handler-ladder rewrite and the contract changes that go with it. Everything
filed carries `status::grooming` until it has been through grooming.
What stays here is the reasoning a tracker has no place for.
## Three defects were recorded only downstream
Found while moving this file and `libakstdlib`'s into their trackers. Each is a
defect **in this library**, each was written down in a *consumer's* TODO file, and
none had an entry here:
| Issue | What it is | What it costs downstream |
|---|---|---|
| #14 | `IGNORE()` logs a context and never releases it | `libakstdlib`'s `aksl_tree_iterate` open-codes log-then-release by hand; every `IGNORE()` in `libakgl`'s `CLEANUP` blocks is a leaked slot |
| #15 | The `coverage` target is not namespaced when embedded, though `mutation` is | `libakstdlib` shadows `add_custom_target` to embed this library at all |
| #16 | No `akerrorConfigVersion.cmake` is installed | No consumer can ask `find_dependency(akerror)` for a version floor |
**That is the finding worth keeping, more than the three defects.** A library whose
consumers record its defects in their own files has no way to see them: each
consumer knows one, nobody sees the pattern, and the workaround gets written three
times. The tracker is now the place, and a consumer filing upstream costs them one
issue instead of one workaround.
## Why the handler ladder is a `3.0.0` change and not a patch
`PROCESS`/`HANDLE`/`HANDLE_GROUP`/`HANDLE_DEFAULT`/`FINISH` compile to a `switch`,
which restricts status matching to integer constant expressions. That is what makes
`HANDLE`-level aliasing undetectable (#4): two components can compile the same
integer into a `case` label without reserving a range or registering a name, and
**ownership enforcement never sees it, because it covers naming and the `case` label
never reaches the library.**
Rewriting it as an `if`/`else if` ladder would allow matching on ranges or
predicates and let a handler resolve a code through its owner. It also touches **the
most load-bearing code in the library and every consumer's error handling at once.**
One thing it would *not* fix, recorded so nobody expects it to: the "don't use
`CATCH` or `FAIL_*_BREAK` inside a loop" hazard comes from exiting via `break`, not
from `switch`.
## Why a copied `akerr_ErrorContext` is a trap
Recorded because the struct looks copyable and is not, and #11 exists only if a
consumer ever needs it.
- **`stacktracebufptr` is self-referential.** `akerr_ErrorContext c = *src;` leaves
the copy's cursor pointing into the source's buffer, so the copy logs correctly
and then **corrupts a slot it does not own** the first time anything appends.
- **`arrayid` is restored after the wipe** in `akerr_release_error()`
(`src/error.c:395-398`), so a copied id makes the destination **impersonate the
source's slot for the life of the process.**
If it is ever built, the shape takes the destination as a parameter rather than
allocating it: an allocating copy could fail on pool exhaustion, and **reporting
that failure needs a pool slot**, so it would have to abort — a third `exit()` site
in a library that deliberately has two.
## Why validating more inputs lowers branch coverage
Branch coverage on `src/error.c` sits just above its 50% gate, and the gate is set
where it is on purpose.
Every `FAIL_*` site carries about **six** branch outcomes of error-construction
machinery (`ENSURE_ERROR_READY`, `AKERR_STACKTRACE_APPEND`) that only run when that
specific failure fires. **Every `PASS` site around a call that cannot fail carries
about twenty-five.**
So adding a defensive check lowers the ratio by construction. **Before adding one,
expect to add a test that drives it**, as `tests/err_copy_string.c` does.
## Why the mutation score is a floor
`scripts/mutation_test.py` configures each mutant with the default CMake options,
so **a mutant that only breaks under concurrency is judged by a suite running
without ThreadSanitizer.**
Measured: deleting the pool's `akerr_mutex_lock()` call survives the run, **even
though it is a real race.** Rebuilt and run directly, that mutant fails
`tests/err_threads_pool.c` in 4 of 10 runs, and fails under `scripts/thread_test.sh`
in 5 of 5.
**81.2% is therefore a floor for that category, not a verdict.** #10 is the
passthrough that would fix the measurement, and it belongs behind a flag: a
TSan-instrumented suite per mutant costs roughly 6s instead of 0.4s.
## Why a registered name is returned by pointer
Make the error pool and status registry thread safe Every entry point may now be called from any thread. akerr_init() runs exactly once however many threads race into it, the pool hands each slot to exactly one thread, and reservations, registrations and lookups are serialized against each other. One recursive lock covers both tables (src/lock.h, private). Recursive because raising an error re-enters the library -- FAIL needs a pool slot and a status name -- and single because two locks would mean an ordering to get wrong. Registry bodies that use the early-returning FAIL_*_RETURN macros are split into *_locked functions behind wrappers that take and release the lock on one path; consumer callbacks are never called under it. This is an ABI break, hence 2.0.0 and SOVERSION 2: - akerr_next_error() now returns a context that already holds its reference. Finding a free slot and claiming it has to be one operation, or two threads scanning at once are handed the same slot. ENSURE_ERROR_READY no longer increments. - __akerr_last_ignored is thread-local, as is the last-ditch context used to report akerr_release_error(NULL). The threading backend is chosen at configure time by AKERR_THREADS (auto, pthread, none). auto fails the configure when it cannot find POSIX threads rather than quietly building a library that reports itself thread safe and is not. generrno.sh stamps the decision into the generated header as AKERR_THREAD_SAFE, so a consumer cannot disagree with the library about it. Tests: err_threads_init, err_threads_pool and err_threads_registry assert exclusive slot ownership, exactly one winner for a contested range, and every registered name readable back under contention. AKERR_SANITIZE builds the library and the tests with any sanitizer; scripts/thread_test.sh runs the suite under ThreadSanitizer and CI runs it. Removing the pool lock makes both the sanitizer and the plain assertions fail, so the tests are not vacuous. Documented in README.md and UPGRADING.md, including what this does not cover: renaming a status while another thread looks it up, and which of two simultaneous unhandled errors sets the exit status. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Andrew Kesterson <andrew@aklabs.net>
2026-07-31 08:31:22 -04:00
`akerr_name_for_status(status, NULL)` returns a pointer into the registry rather
than a copy, **which is what makes it usable from inside `FAIL`** — it needs no
buffer and no error context of its own.
The cost is that renaming a status is not safe against a concurrent lookup (#7):
every other registry operation is serialized, and this one cannot be, because **the
reader is outside the lock by the time it reads the characters.** Documented in
`docs/thread-safety.md` and `UPGRADING.md` as "register names during
initialization".