Retain WinMM buffers when teardown cannot reclaim them - #131
Blackspirits wants to merge 1 commit into
Conversation
Blackspirits
left a comment
There was a problem hiding this comment.
Final adversarial review on frozen HEAD eb9ccb347022263004d8f0d79bd623c247bc87ae after CI SubtitleEdit#264.
Rechecked WinMM teardown ownership across reset failure, partial header-unprepare failure, close failure, event-handle failure, repeated Dispose, and Open-after-failed-cleanup. Buffer/header memory is now released only after WinMM has successfully reset, unprepared every header, and closed the device. Any earlier native failure retains the complete device/header/data/event boundary for retry. Retrying after a partial unprepare pass is safe because already-unprepared headers can be passed to waveOutUnprepareHeader again. If only CloseHandle fails after the waveform device has been closed, only that event handle is retained.
No blocker or regression found in the changed set.
CI SubtitleEdit#264 passed on this exact HEAD: SeConv 488 passed / 2 skipped / 0 failed; LibUiLogic 905/0/0; LibSE 2017/0/0; UI 5233 passed / 9 skipped / 0 failed. Build: 0 errors / 7 pre-existing warnings. Retry step skipped. PR remains draft; no merge performed.
Purpose
Dependent Windows/WinMM follow-up to #129 ("Fail closed when WinMM output setup is incomplete").
This tranche intentionally uses #129 as its base because it extends the same native-resource ownership boundary from setup into teardown. It does not alter the playback-position logic from #126 or FFmpeg decode/demux behavior.
Finding fixed
WinMM teardown freed native buffer memory even when the driver had not released it
WaveOutAudioSink.CloseCore()previously ignored every native teardown result:waveOutReset();waveOutUnprepareHeader()for every header;waveOutClose();_headersand_dataand close the event.Microsoft's WinMM contract does not permit that assumption:
waveOutReset()returns an error on failure; only successful reset guarantees pending buffers are marked done/returned;waveOutUnprepareHeader()returnsWAVERR_STILLPLAYINGif a data block is still in the queue, and Microsoft explicitly requires the driver to be finished with a buffer before it is unprepared/freed;waveOutClose()returnsWAVERR_STILLPLAYINGwhile buffers remain queued.Therefore a failed reset/unprepare/close could be followed by freeing memory still referenced by the audio driver — a native use-after-free during Dispose/error cleanup.
Fix
CloseCore()is now a retryable fail-closed transaction:waveOutReset()before proceeding;waveOutUnprepareHeader()call to succeed before closing the device;waveOutClose()before clearing the device handle or freeing buffer memory;_headersand_datafreed;CloseHandle()is also checked; if it fails, only the event handle is retained for retry because the waveform device and buffer memory are already safely released;Open()refuses to initialize a replacement device if cleanup of a previous one could not complete safely.Retry after a partial unprepare pass is valid: Microsoft documents that unpreparing a header that is already unprepared has no effect and returns success.
Scope / branch state
4e262966119437f3e90f87693ff279c65dc20dcbeb9ccb347022263004d8f0d79bd623c247bc87aea3855e5fb200e8c5390704cf67df5b0485c355bfAI assistance: ChatGPT was used for adversarial WinMM teardown/ownership review, Microsoft API contract verification, controlled-leak/retry design, and diff review.
Final CI
Authoritative run: SubtitleEdit#264 on
eb9ccb347022263004d8f0d79bd623c247bc87aeFinal adversarial review was performed on this exact HEAD after CI; no blocker found.