feat: validate minimum stack size and guard destroy against a running fiber - #6
Merged
Merged
Conversation
A stack_size below the initial setup frame made initStack write past the low end of the allocation and silently corrupt the heap. Add a public min_stack_size (one page) and return error.StackTooSmall before allocating anything when options.stack_size is below it. The error is additive to create's inferred error set; no existing caller passes a sub-page stack.
Assert a fiber is not .running before destroy frees its stack (freeing the in-use stack is undefined). Document the state preconditions of create, destroy, resumeFiber, and yield so callers know the contract the debug asserts enforce.
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
1 participant
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.
Summary
Harden the API against two misuse cases that previously led to silent memory corruption or UB: a stack size too small to hold a fiber, and destroying a running fiber. Pure Zig — no assembly changes. (Phase C sub-project C1.)
Changes
pub const min_stack_size = 4096(one page).createnow returnserror.StackTooSmallwhenoptions.stack_size < min_stack_size, checked before any allocation. A too-small stack previously madeinitStackwrite past the low end of the allocation (~280 bytes of setup frame on Windows) and silently corrupt the heap. The error is additive tocreate's inferred error set — no existing caller passes a sub-page stack, so nothing breaks.destroyguard —std.debug.assert(self.state != .running); freeing the stack of a running fiber is UB..ready/.suspended/.doneremain valid to destroy.create,destroy,resumeFiber, andyieldstating the state contract the debug asserts enforce.Notes
Out of scope (later sub-projects): a runtime guard page for overflow detection is C3 —
min_stack_sizeguards setup, not a fiber overrunning its stack while running. InReleaseFast/ReleaseSmallthe asserts are compiled out (standard Zig contract for misuse guards); Debug and ReleaseSafe (both in CI) keep them.Test Plan
zig build testpasses across all four optimize modes locally (Windows)zig build examples,zig fmt --check .passFailingAllocatorproving the check precedes allocation); the minimum size runs a fiber to.done