ci(soak): run go test -race ./... by default — the only shape that has reproduced the crash - #60
Closed
deadpoets wants to merge 1 commit into
Closed
ci(soak): run go test -race ./... by default — the only shape that has reproduced the crash#60deadpoets wants to merge 1 commit into
deadpoets wants to merge 1 commit into
Conversation
… reproduced The soak looped a prebuilt single-package binary. That ran 1250 iterations and found nothing, while two ordinary CI runs reproduced the crash — so the soak was measuring the wrong thing and its clean results were not the evidence they looked like. ci.yml runs `go test -race ./...`, which compiles and runs the core and redact test binaries CONCURRENTLY. For a bug whose signature is a stray write landing in runtime free-list caches, "what the allocator hands back next" is the entire question, and concurrent processes change that answer. The prebuilt-binary loop is faster per iteration but is not the same experiment. `mode` selects between them and defaults to gotest, because a default that is fast and has never reproduced is worse than a slow one that has. Header records the second sighting: acquireSudog found a released sudog with elem != nil, where the first was the goroutine free list with a non-power-of-two stack size. Both are per-P free-list caches in an impossible state, and in both the crashing frame is innocuous — the goroutine trips over earlier damage. Also notes the discriminator that fell out of sighting 2: making a field NON-nil cannot be done by a zero-wipe, so the canary fill (random pattern) and ordinary buffer writes are better suspects than secureWipe.
Owner
Author
|
Landed on The soak now defaults to Rebased locally so the commit carries your SSH signature; different SHA, hence the manual close. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The soak was measuring the wrong thing, and its clean results were not the
evidence they looked like.
What happened
The workflow looped a prebuilt single-package test binary. Across 1250
iterations it found nothing — while two ordinary CI runs reproduced the
crash:
fatal error: stack not a power of 2(goroutine free list)fatal error: acquireSudog: found s.elem != nil in cache(sudog cache)ci.ymlrunsgo test -race ./..., which compiles and runs the core andredacttest binaries concurrently. For a bug whose signature is a straywrite landing in runtime free-list caches, what the allocator hands back next
is the entire question — and concurrent processes change that answer. The
prebuilt-binary loop is several times faster per iteration, but it is not the
same experiment.
modenow selects between them and defaults togotest, because a defaultthat is fast and has never reproduced is worse than a slow one that has.
The signature is now specific
Both sightings are per-P runtime free-list caches found in an impossible
state:
gfreesudogelem != nilIn both, the crashing frame is innocuous —
TestCapabilities_String, thent.Parallel(). The goroutine merely trips over damage done earlier. That is astray write into memory the Go runtime owns.
One useful discriminator fell out of sighting 2. Making a field non-nil
cannot be done by a zero-wipe — that would set it to nil. So
secureWipeis aweaker suspect than the canary fill (which writes a random pattern) and ordinary
buffer writes. Recorded in the workflow header.
What this does not claim
It does not claim #56 fixes this. That PR's tree is not what crashed, and its
defect wipes another live, mapped secmem buffer — destroying that buffer's
secret without obviously reaching Go-owned memory. A soak against the #56 branch
is running to see whether the rate moves.
Validated: bash syntax checked over the extracted 54-line script, YAML parsed.