Invalid for libakstdlib: PASS/CATCH do release; the one demonstrable leak is libakerror's IGNORE(), fixed by libakerror #21 #37
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This issue is invalid and should be closed. Nothing in libakstdlib is missing or misshapen. Rewritten a second time after @andrew's review (comment 1094).
Every claim this ticket has been filed on has now been withdrawn:
AKERR_STACKTRACE_APPENDworks as he wrote it.NULLis the test.return aksl_remove(name) == NULL;,(void)aksl_remove(name);) were written by me for the measurement. No consumer writes them. A footgun that only fires in code nobody writes is not a defect report, and the burden was correctly placed on this ticket to produce a real call site.Andrew is right about
PASSandCATCHVerified against
include/akerror.tmpl.hat libakerror5eaa956(the commit akbasic pins):DETECT(:417) is guarded byif ( __err_context != NULL ), so aNULLreturn does nothing. Correct.CATCH(:426) breaks toCLEANUP;PROCESS(:445) dispatches on status;HANDLE/HANDLE_GROUP/HANDLE_DEFAULT(:451/:457/:463) sethandled = true;FINISH(:476) ends inRELEASE_ERROR(__err_context).PASS(:429) hands the context up to a frame that does the above.There is no user intervention required and no leak on either path. That is the whole of what this issue claimed, and it was wrong.
The one demonstrable leak is on a third path, and it is not libakstdlib's
Andrew asked for a real consumer site. There is one, and it is
IGNORE()— which is neitherPASSnorCATCH, and which logs the context without releasing it:akbasic
main(330d731) wraps anaksl_*call in it at nine checked-in sites:src/main.c:207·src/runtime_disk.c:64,347,349,428,476,677·src/runtime_commands.c:654,705This is already known in this repository.
libakstdlib/src/stdlib.c:1294avoids the macro for exactly this reason, and says why:libakstdlib uses
IGNORE()nowhere. akbasic uses it in nine places where the wrapped call is anaksl_*.Measured, end to end, on a stock build
cmake -S . -B build && cmake --build buildon akbasic330d731with its pinned submodules, no patches./dev/fullstands in for the full disk thatakbasic_cmd_dsave's own comment names as the motivating case.Exit status 1 on the 129th iteration.
AKERR_MAX_ARRAY_ERRORis 128, so the count is exactly the pool. Line 50 never runs. A second, independent site reproduces identically:DSAVE "/dev/full"x200 (runtime_commands.c:705)exit(1)on the 129thCOPY "f.txt","/dev/full"x200 (runtime_disk.c:349)exit(1)on the 129thThe BASIC program is five lines and uses no unusual construct. The failure it needs is a disk that is full — which is the exact condition
akbasic_cmd_dsavewas changed to report in the first place.The fix exists and is already open
Rebuilding the same akbasic against libakerror PR #21 (
4fe7571, "Release ignored error contexts"), which adds one line toIGNORE:5eaa956DSAVE "/dev/full"x200COPY "f.txt","/dev/full"x200akbasic's own suite is 112/112 against that branch.
Recommendation
Close this issue. No libakstdlib change follows from any version of it.
The action is in libakerror, and it is already written: merge libakerror #21. Until it lands, the workaround is the one
libakstdlib/src/stdlib.c:1294already uses — log by hand, thenakerr_release_error()— and akbasic's nine sites are akbasic's to convert.Two unrelated libakerror observations
Neither defends this ticket; recorded so they are not lost.
docs/uncaught-errors.md:17says "By design users may explicitly ignore an error code from a function marked withwarn_unused_resultby explicitly casting the return tovoid." Under gcc 15.2 that is false —(void)f()still raises-Wunused-result— and the cast holds a pool slot besides (measured: 5 discards, 5 slots). The sentence describes clang's behaviour and reads as sanctioning an idiom that neither compiles clean nor releases.HANDLE/HANDLE_GROUP/HANDLE_DEFAULTresetstacktracebufptrto offset 0 without clearingstacktracebuf. Inside the handler the buffer still reads intact, but the cursor is at 0, so the nextAKERR_STACKTRACE_APPENDoverwrites from the start. Measured: a caller's annotation is readable in the handler and gone after one further append.Effect on #38
Unchanged by the above; @andrew has ruled on #38 separately (its comment 1095) and it is being rewritten to record that ruling.
Filed by Tachikoma (Claude Code, Opus 5, 1M context)
@tachikoma
AKERR_STACKTRACE_APPEND(e, "SCRATCH could not delete \"%s\"", name)... but setting and reading strings is a dumb way of handling errorsIf an aksl_* function returns NULL, it worked. What's the problem?
A caller cannot add its own context to a wrapper's error, so it raises and discards insteadto A caller CAN annotate and CAN test success; the real hazard is that discarding a context leaks the pool@andrew You are right on both counts. I have retracted both claims and rewritten the body; the ticket now recommends closing itself. Retitled too, since the old title asserted the thing that turned out to be false.
On
AKERR_STACKTRACE_APPEND. I filed "no way to annotate" without compiling it, and it works exactly as you wrote it. Realaksl_remove()failure, caller appends, propagates up two frames throughPASS, read at the top:One caveat I did find, small and libakerror's:
HANDLE/HANDLE_GROUP/HANDLE_DEFAULTresetstacktracebufptrto the start of the buffer without clearing it, so an annotation looks intact after aHANDLEbut the next append overwrites it from offset 0.On
NULLmeans it worked. Also right, and I should not have written otherwise. But testing it and disposing of the context are different problems, and that is where the actual bug is. This compiles under-Wall -Wextrawith no-Wunused-resultwarning, because theNULLtest counts as using the value:and it holds a pool slot per failure. Measured, 200 consecutive failures each:
RELEASE_ERROR(e)e->handled = true; IGNORE(akerr_release_error(e))return aksl_remove(name) == NULL;exit(1)on the 128th(void)aksl_remove(name);exit(1)on the 128thIGNORE(aksl_remove(name));exit(1)on the 128thsizeof(akerr_ErrorContext)is 37296 and the pool is 128 slots, so that is 4.77 MB going dry andENSURE_ERROR_READYcallingexit(1). The macro namedIGNOREis the one that kills you, it appears nowhere in libakerror's README ordocs/, anddocs/uncaught-errors.md:17recommends the(void)cast — which gcc does not honour forwarn_unused_resultand which leaks too. I rebuilt libakstdlib against libakerror #21 and it fixes theIGNORErow cleanly (pool stays at 0 across 200). The(void)row is unfixable and stays fatal.On "setting and reading strings is a dumb way of handling errors." Agreed, and I think the sharper version is that at these sites you are not actually missing a structured channel — you already have one and akbasic already uses it.
akbasic/src/error.c:20-37reserves a status range and registers eightAKBASIC_ERR_*names. Andaksl_removeraisesAKSL_ERRNO_OR(AKERR_IO)with"%s", pathname, so the context arrives carrying->status == ENOENT,->message == "t_nosuch.txt",->function == "aksl_remove". Every part ofSCRATCH could not delete "t_nosuch.txt"except the verb's own name is already in a machine-readable field, and the verb name is in the frame above. What makes the string feel load-bearing is only thatakbasic/src/runtime.c:472,495,1275,1376,1818render->messagealone. Going further — typed(kind, value)payload slots — costs either more of a struct that is 37 KB because everything is a fixed string buffer, or the first heap allocation on a path that is currently allocation-free. I do not think that trade pays at this size.What I think should happen, and the only thing I need a ruling on: close #37, and let me file two against libakerror — (1) document the discard contract, since the failure mode is a process death at the 128th error rather than a compile error, and (2) the
HANDLEcursor-reset caveat. #21 already covers the third. Say the word and I will file them; I have not touched anything yet.#38 is affected too. Its "a
boolpredicate has no way to call one" framing is wrong the same way — aboolpredicate can call a wrapper andRELEASE_ERRORthe context. What survives there is different and I still think worth deciding: the cost inscanner.c's per-character loop, and that aboolreturn has nowhere to distinguish "not equal" from "the comparison failed". I would narrow #38 rather than close it, but I have left it untouched pending your call.@tachikoma
There is no bug.
PASS()andCATCH()already handle all of this complexity. If it's NULL, nothing happens. If it's non-NULL, it goes to aHANDLE {}block, and releasing happens there, without user intervention. There is no leak.Where are you getting this? Unless you can point me to something akbasic (or some other) consumer in doing in this regard that creates a demonstrable leak, I'm going to close this.
A caller CAN annotate and CAN test success; the real hazard is that discarding a context leaks the poolto Invalid for libakstdlib: PASS/CATCH do release; the one demonstrable leak is libakerror's IGNORE(), fixed by libakerror #21@andrew You are right, and this ticket should be closed.
PASSandCATCHdo release without user intervention — I read the expansions this time:DETECTis guarded byif (e != NULL),HANDLE*setshandled, andFINISHends inRELEASE_ERROR. The idioms I measured last time were ones I wrote for the measurement. No consumer writes them, and that made the whole table an argument about nothing. Body rewritten to say so; nothing in libakstdlib changes.You asked me to point at a consumer doing this. There is one, and it is on neither of the paths you described — it is
IGNORE(), which logs the context and does not release it:The search.
grep -rn "IGNORE(" src/ include/ tests/ | grep -v NOIGNOREon akbasic330d731, then the same over libakstdlib'ssrc/andtests/, plus(void)-cast, bare-expression and== NULL-in-a-bool-function sweeps over both. libakstdlib: noIGNORE()at all, no(void)discards, nothing. akbasic: nine sites wrap anaksl_*call inIGNORE—src/main.c:207,src/runtime_disk.c:64,347,349,428,476,677,src/runtime_commands.c:654,705.The reason libakstdlib has none is written down in libakstdlib.
src/stdlib.c:1294, in the tree traversal'sCLEANUP:So this is your own finding, already acted on in one repo and not the other.
Measured on a stock build — akbasic
330d731, its pinned submodules,cmake -S . -B build && cmake --build build, no patches./dev/fullstands in for the full disk thatakbasic_cmd_dsave's own comment names as the case it was changed to report:128
** IGNORED ERROR ** 28 (No space left on device)lines, thensymtab.c:akbasic_symtab_get:110: Unable to pull an error context from the array!and exit 1 on the 129th iteration. Line 50 never runs.COPY "f.txt","/dev/full"x200 does the same thing atruntime_disk.c:349— 128, then dead.Your own open PR fixes it. Rebuilding that same akbasic against libakerror #21 (
4fe7571), which adds oneRELEASE_ERRORline toIGNORE: both programs survive all 200, log all 200, exit 0. akbasic's suite is 112/112 against that branch. So the whole action here is merging #21 — and closing this ticket, which never had anything to do with libakstdlib.Two unrelated things I tripped over in libakerror while checking, not a defence of anything above:
docs/uncaught-errors.md:17says a(void)cast is the by-design way to ignore awarn_unused_resultreturn, but gcc 15.2 warns on it anyway (that spelling is clang's), and it holds a pool slot besides. AndHANDLE/HANDLE_GROUP/HANDLE_DEFAULTresetstacktracebufptrto offset 0 without clearing the buffer, so inside a handler an annotation still reads intact but the next append writes over it from the start.