Repository navigation
Fix AudioOut release-notifier lost wakeup - #855
Closed
thehobbit85 wants to merge 1 commit into
Closed
thehobbit85 wants to merge 1 commit into
thehobbit85 wants to merge 1 commit into
Conversation
This was referenced Nov 1, 2026
thehobbit85
force-pushed
the
codex/fix-audio-release-wakeup
branch
from
November 1, 2026 12:33
e0bdbbc to
db8ea2f
Compare
This was referenced Nov 1, 2026
thehobbit85
force-pushed
the
codex/fix-audio-release-wakeup
branch
from
November 1, 2026 20:30
db8ea2f to
ccb4029
Compare
seedds
added a commit
that referenced
this pull request
Nov 1, 2026
…855) The notifier cleared notify_armed with a release-only store, then read notify_pending. A producer could set pending and see the old armed value, so it did not post, while the notifier read the old pending and went back to sleep: PCM played and the buffer was released, but the guest's event never came. Apple-silicon code is stlr then ldapr, which permits this. Disarm with an acquire-release exchange. If the producer's exchange came first, the notifier acquires its pending store; otherwise the producer posts, or finds the notifier already re-armed with its own post. t_audio_stats gains 100,000 individual handoffs. 400 runs each, old notifier against the fix: plain 8 failed against 0, ASan/UBSan 74 failed against 0. Co-authored-by: Eliran Zach <4012041+thehobbit85@users.noreply.github.com>
Owner
|
Thanks, this is landed on main as c76a424, with you credited as co-author. The code, test and docs are unchanged from this PR. I checked it independently before landing:
No game run was done: the change is one atomic operation with no timing change, so I judged the regression risk low. No gameplay result is claimed. |
3 tasks done
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.
AudioOut can consume PCM and place a buffer on its released queue while its notifier sleeps without signaling the guest. This caused the earlier #691 sanitizer failure; the same production code is on main.
The notifier disarmed with a release-only store, then rechecked a separate pending flag. A producer could set pending, observe the previous armed value and omit its semaphore post; the notifier could miss that publication and sleep. Local ASan reproduction captured correct PCM and a queued release with
pending=1, armed=0. Temporary phase instrumentation confirmed the helper remained at its semaphore wait; a diagnostic post immediately delivered the release. That instrumentation is not in this PR.Disarm using an acquire-release exchange. An earlier producer exchange is acquired before the pending recheck; otherwise the producer or the notifier's re-arm owns the semaphore post. This changes no callback timing, PCM processing, queue limits, logging policy or wait deadlines. The original 22 fixture checks remain, with four added checks for 100,000 individual handoffs and diagnostic failure details. The reference/symptom entry explains the cause; a narrow impact rule maps the asynchronous notifier to AudioOut handles.
Current integration and validation
Rebased the unchanged repair onto pinned main
cd859c38a8f8890ddbb57a23d6f45e23ba9e6651, producingccb4029a252d2840a3d2545bea6acaa07bfc4a83. Main still uses the release-only disarm store, so the wakeup fix remains needed. Canonicalcodex/fix-audio-release-wakeuptracks the same existingfork/codex/fix-audio-release-wakeupbranch. Main ancestry, range-diff and whitespace checks pass; all applicable discussion/review/inline feedback was read with pagination and no new request was found.Fresh on this head: all 26 native audio-statistics checks and 17 documentation/usage tests pass. Audio production, fixtures, headers, generated profile and target inputs match freshly checked #691
d0e54fa7a073ad5d3464da1de11a08b55df3efdb; its native, ASan/UBSan and TSan statistics runs each pass 26 checks. Nine LLVM-IR comparisons prove that the two new, unused header declarations do not alter audio code generation. These checks support reuse of the original stress evidence below, without claiming that the 300-run campaign was repeated.Fresh game discovery still finds only Z-A, without a smoke route or baseline; four selected games remain unavailable. No game or extract rebuild was run. This pass's shared upstream graphics/filesystem coverage is recorded separately; historical shader-only comparisons do not establish gameplay. Local Xcode27.0/Metal.921 and unavailable LLVM23 limitations remain. Current-head CI is pending; previous-head successes are historical.
Historical repair validation
Validation on
e0bdbbc49ca00b5f65a146083cf3fee934c2d18a, based on mainc0f341335b56ee61af5827b51349e68bddb65c61:The shared repair is separate from #691's documentation feature and #221's controller feature; both now include it as a CI prerequisite. No baseline expansion, suppression, blind retry or timeout increase is used.
Available-game follow-up: a separate Z-A check completed successfully in 25.9 seconds. It reused the same compatible cached chain for both revisions (zero shader build time), freshly compared 28,603 shaders and checked 26,051 pipeline interfaces: no changed MSL/reflection/variants, newly failing shaders or interface errors. There is no smoke route, so no boot, gameplay, audio-runtime, frame-time or device result is claimed. The four unavailable games remain unvalidated. Existing caches and the pinned-toolchain policy were preserved.
The harness report below uses its standard
check (full)label; its empty boot/performance columns and chain-only timing delimit the actual coverage:regress.py check (full): PASS
Baseline
c0f341335b56ee61af5827b51349e68bddb65c61c0f3413, candidatee0bdbbc49ca00b5f65a146083cf3fee934c2d18ae0bdbbc; Apple M2 Max, macOS 26.6, Xcode 27.0; 25.9 s0100F43008C44000LegendsZA
0100F43008C44000: PASSCurrent published head
ccb4029a252d2840a3d2545bea6acaa07bfc4a83has all 7 applicable CI checks successful, including every applicable workflow and job. Verified on 2026-11-01 against exact head/run/job provenance and recorded maincd859c38a8f8890ddbb57a23d6f45e23ba9e6651. Historical results and local limitations above remain separate. Workflows: lint-c, tests.