fix(segment): a failed re-commit is a failed span, not unbacked memory - #44
Merged
Merged
Conversation
With purging on and decommit enabled, a freed span is decommitted and reuse re-commits it, but span_recommit discarded os::commit's result. Windows has no overcommit: MEM_COMMIT fails once the system's commit is exhausted, and the span was carved anyway, so the page layer's first store faulted (access violation in page_extend, seen in the MATA desktop app with secure on and purge_delay = 0 at 98.5 % of the commit limit). - span_recommit returns whether the span is backed; on failure it stays on the free list, still marked purged, and span_alloc returns null so the caller moves to another segment. - segment_free / huge_free: restore_for_reuse replaces the two inline protect+commit pairs whose results were dropped. A segment the OS will not restore is never recycled: released to the OS, or retired in place when it is arena chunks (arena::owns tells the two apart). - stats::commit_failures(): process-wide counter, printed by print_process, to tell this mechanism from a missing re-commit on the next crash. Test: segment::recommit_tests, via a cfg(test) thread-local "fail the next N commits" switch in os::commit. Poisoned: with the span_alloc check disabled it fails at "a span whose re-commit failed was handed out". Unsafe census 965 -> 973 (+3 shipped, +5 test), recorded in UNSAFE.md. Co-Authored-By: Claude Opus 5.5 (1M context) <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
2 participants
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.
Fixes mechanism A of
docs/plans/recommit-failure-ignored.md: an access violation inpage_extendseen in the MATA desktop app (Windows,secureon,purge_delay = 0, machine at 98.5 % of its commit limit).Defect.
span_recommitdropped the result ofos::commit. Windows refusesMEM_COMMITwhen the system's commit is exhausted, so a purged span whose re-commit failed was carved anyway, and the first store faulted.segment_free/huge_freedropped the results of their protect + commit restore the same way before recycling a segment.Change.
span_recommitreturns whether the span is backed; on failure the span stays on the free list, still marked purged, andspan_allocreturns null (both callers already move on to another segment).restore_for_reusereplaces the two inline restore pairs. A segment the OS will not restore is never recycled: released to the OS, or retired in place if it is arena chunks (arena::owns).stats::commit_failures(), process-wide, printed byprint_process.Exposure. Purging on (
purge_delay >= 0) withpurge_decommitson, on Windows, under commit exhaustion. The default (purge_delay = -1) never purges.Verified (Windows). New test
segment::recommit_testsvia acfg(test)thread-local commit-failure switch; poisoned (fix disabled) it fails at "a span whose re-commit failed was handed out". Lib + 10 integration suites pass (88), withsecuretoo (71); clippy-D warnings(secure); fmt;cargo check --target wasm32-unknown-unknown; unsafe census 965 -> 973 recorded inUNSAFE.md.Not covered by a test: the two whole-segment release sites. Still open: mechanism B (a reuse path that skips the re-commit); the new counter is what will tell A from B next time.
Intended release: 2.2.3.
🤖 Generated with Claude Code