Repository navigation
fix(pool): release the last job's arrays; no writes after a recovered caller panic - #33
Merged
Merged
Conversation
… caller panic propagates Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Security audit of the helper pool added in v0.6.0 (#31). Two defects, both fixed. Each fix comes with a test that fails without it.
1. Retention of the last operation's arrays
pool.jobkept the last job, and its closure holds the operands. The arrays of the last parallel operation therefore could not be garbage-collected until another parallel operation replaced the job. That is up to the largest array, for as long as the program runs.noJob(w = 0, so no helper takes it). A helper that loaded the real job just before finds every block taken and drops it on return.TestPoolDoesNotRetainLastJobuses aweakpointer plusruntime.GC(). It fails on v0.6.0. A first version passed on the defective code because its closure captured a variable that the test then set to nil; it now captures a function parameter.2. Writes after a recovered panic in the caller
Since v0.6.0 the caller runs blocks itself. If
bodypanicked there (a userMapfunction, for example), the panic propagated while the helpers kept writing into the arrays of a caller that might recover it.TestPoolCallerPanicStopsTheHelperschecks that no block starts after the panic propagates and that the pool keeps working. Removing the wait makes it fail ("1 blocks ran after the panic propagated").runningtouches zero between two blocks, which no test can target reliably. That reasoning is written in the code.Map's doc now says so.Checks
-race, coverage 100%.go veton linux amd64/ppc64le/loong64/386/riscv64.govulncheck: none.Version: v0.6.1.
🤖 Generated with Claude Code