| Comment: | build: --release means ReleaseSafe, and pin it
src/ carries 1086 std.debug.assert calls. They are how a detected inconsistency becomes a crash instead of a durable write -- TigerStyle's 'downgrade catastrophic correctness bugs into liveness bugs' -- and that trade exists only while the assertion does. ReleaseFast and ReleaseSmall compile every one to nothing, so a binary built that way converts each guarded corruption into an unguarded one, in a store where a bad frame is permanent and a crash is not. Measured, not assumed. Read-side verbs are indistinguishable between the two modes: status, log and stat all sit inside noise on a 5 MB store. zit pack is not: 599ms fast against 15.2s safe, about 25x, because the delta encoder's best-of-N search is a tight per-byte loop where a bounds check is a large share of the work. That argues for setRuntimeSafety(false) on that measured loop -- local, reviewable, over code the fuzz and DST lanes already cover -- rather than stripping assertions from every other path in the binary. Not taken here. Note the option surface changes: stating a preference replaces the -Doptimize enum with a -Drelease bool, so the deliberate escape hatch is the top-level --release=fast. -Doptimize=ReleaseFast no longer parses. ci_test pins the preference, and says that if the project ever decides to ship unchecked, the pin should be deleted in that same commit so the decision is visible in history rather than arriving as drift. |
|---|---|
| Timelines: | family | ancestors | descendants | both | trunk |
| Files: | files | file ages | folders |
| SHA3-256: |
4379187a10bdb238094f31941510442e |
| User & Date: | dmestas on 2026-07-30 21:20:03.343 |
| Other Links: | manifest | tags |
|
2026-07-30
| ||
| 21:47 |
release_assert: a guard against assert used as input validation
std.debug.assert is right for an internal invariant and wrong for a rule about data the function did not produce, and it is wrong in two different ways. Under the ReleaseSafe default it survives and PANICS: on the write path that is the trade we want, a crash over a durable bad frame, but on a peer-reachable path it is a remote denial of service. Under an explicitly chosen --release=fast it compiles to nothing and the input flows through unvalidated. One fix, a checked error, closes both. Flags an assert only when all of: the fn is pub, returns an error union so a rejection was already available, the assert names one of its own parameters, and no earlier return in the body already rejects on that parameter. That last rule is what makes it signal -- delta.applyCopy returns MalformedDelta and then asserts to restate it, which is the CORRECT shape and must not be flagged. The baseline is keyed on file, function and COUNT, never on line numbers: line numbers churn on every edit above them, while a count still catches a second bad assert added to a function that already has one. It fails in both directions, so a fixed finding must be removed from the list and the list can only shrink. Existing findings are recorded, not fixed. Triage belongs in issue 19, where the ReleaseSafe default already reduces the write-path subset to crash-instead-of-corrupt and leaves the sync-side ones as the live exposure. check-in: 949e2b5542 user: dmestas tags: trunk | |
| 21:20 |
build: --release means ReleaseSafe, and pin it
src/ carries 1086 std.debug.assert calls. They are how a detected inconsistency becomes a crash instead of a durable write -- TigerStyle's 'downgrade catastrophic correctness bugs into liveness bugs' -- and that trade exists only while the assertion does. ReleaseFast and ReleaseSmall compile every one to nothing, so a binary built that way converts each guarded corruption into an unguarded one, in a store where a bad frame is permanent and a crash is not. Measured, not assumed. Read-side verbs are indistinguishable between the two modes: status, log and stat all sit inside noise on a 5 MB store. zit pack is not: 599ms fast against 15.2s safe, about 25x, because the delta encoder's best-of-N search is a tight per-byte loop where a bounds check is a large share of the work. That argues for setRuntimeSafety(false) on that measured loop -- local, reviewable, over code the fuzz and DST lanes already cover -- rather than stripping assertions from every other path in the binary. Not taken here. Note the option surface changes: stating a preference replaces the -Doptimize enum with a -Drelease bool, so the deliberate escape hatch is the top-level --release=fast. -Doptimize=ReleaseFast no longer parses. ci_test pins the preference, and says that if the project ever decides to ship unchecked, the pin should be deleted in that same commit so the decision is visible in history rather than arriving as drift. check-in: 4379187a10 user: dmestas tags: trunk | |
| 20:21 |
logstore: bound the priority window structurally, not with an assert
gatherCandidates capped the entries ADDED from the caller's priority slice, not the elements read: a duplicate hits ridListed and continues without advancing n, so the fill check never fires and the walk runs the caller's whole array with a linear dedup scan at each step. The only thing holding that back was an assert that the slice fits the window -- and std.debug.assert compiles to nothing in ReleaseFast and ReleaseSmall, so there was no guard in exactly the build where an unbounded scan would matter. Slicing the prefix holds in every mode, and turns an over-length slice from a precondition violation into defined behavior: accepted, tail ignored. Both callers today are inside the window either way. Two tests, both mutation-checked against the old shape. What is NOT pinned is the duplicate-heavy case: dedup leaves the surplus slots to the chronological walk, so the candidate set is identical either way and only the count of elements read differs. That is a cost property, not a behavioral one, and no test through this API can catch it -- the header comment says so rather than letting the pair read as covering it. check-in: bfec65b1a1 user: dmestas tags: trunk | |