Fix silent crash when disposing mpv with calls in flight (#12093)#12237
Closed
niksedk wants to merge 1 commit into
Closed
Fix silent crash when disposing mpv with calls in flight (#12093)#12237niksedk wants to merge 1 commit into
niksedk wants to merge 1 commit into
Conversation
Pressing Done in the TTS window while audio was in flight killed the process instantly with nothing in error-log.txt (reported in #12093). TextToSpeechViewModel.OnClosing disposes the playback mpv instance on the UI thread while PlayAudio's "loadfile" command may still be running on the threadpool (LoadFile issues it via Task.Run, and PlayAudio awaits LoadAudio outside the play lock). DoMpvCommand's handle check and the actual mpv_command call are not atomic against Dispose, so the command could execute on a handle mpv_terminate_destroy had already freed - a native use-after-free that terminates the process before any managed exception handler can log it. Position polls (every 50 ms from the video player control) race disposal the same way. Serialize native calls against disposal inside LibMpvDynamicPlayer with a ReaderWriterLockSlim: every native call enters as a reader (libmpv is thread-safe for concurrent client calls, so readers never block each other), and Dispose enters as the writer, so mpv_terminate_destroy can never overlap an in-flight call. Calls that arrive during/after disposal skip the native call and return their existing fallback values instead of blocking - a position poll never waits on the potentially slow mpv_terminate_destroy. The property getters and the render-context init/render sites are routed through small locked helpers (GetPropertyDoubleLocked, GetPropertyStringLocked, InitializeCoreLocked, CreateRenderContextLocked, RenderContextRenderLocked) so the per-site diff stays mechanical. Co-Authored-By: Claude Fable 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.
Problem
Reported in #12093: pressing Done in the TTS window kills the app instantly — clean process exit, nothing in error-log.txt or the tools log. Both global exception handlers write to error-log.txt, so a silent exit means this is not a managed exception: it's a native crash.
Root cause
TextToSpeechViewModel.OnClosingdisposes the playback mpv instance synchronously on the UI thread, whilePlayAudio'sloadfilecommand may still be running on the threadpool (LoadFileissues it viaTask.Run, andPlayAudioawaitsLoadAudiooutside the play lock).DoMpvCommand's handle check and the actualmpv_commandcall are not atomic againstDispose, so the command can execute on a handlempv_terminate_destroyhas already freed — a native use-after-free that terminates the process before any managed handler can log it. Position polls (every 50 ms fromVideoPlayerControl) race disposal the same way, and the review window's per-playback mpv instances have the identical pattern.Fix
Serialize native calls against disposal inside
LibMpvDynamicPlayerwith aReaderWriterLockSlim:Disposeenters as the writer: it waits for in-flight calls to drain, then runsmpv_render_context_free+mpv_terminate_destroy, which can now never overlap a native call.TryEnterReadLock(0)), so a UI-thread position poll never waits on the potentially slowmpv_terminate_destroy(same concern as the Adding a logo to the output video freezes the interface and it cannot be closed in any way. #11176 freeze).The property getters and render-context init/render sites are routed through small locked helpers (
GetPropertyDoubleLocked,GetPropertyStringLocked,InitializeCoreLocked,CreateRenderContextLocked,RenderContextRenderLocked) so each call-site change stays mechanical.Testing
Note: conflicts trivially with #12235 (same file, adjacent lines in
LoadFile/init paths) — happy to rebase whichever lands second.Part of #12093.
🤖 Generated with Claude Code