hudec117 / Mpv.NET-lib-

.NET embeddable video/media player based on mpv for WinForms and WPF
MIT License
139 stars 36 forks source link

Fix race condition on shutdown event #29

Closed SebastianBecker2 closed 3 years ago

SebastianBecker2 commented 3 years ago

Closes #28 When the event loop receives the shutdown event the event loop task tried to TerminateDestroy the mpv handle and subsequently wait for itself to finish. Instead of calling TerminateDestroy in the shutdown event handler, it relies on the Mpv::Dispose(bool) function to do so. And the event loops stop functions checks if it's called by it's own thread to avoid waiting on itself.

SebastianBecker2 commented 3 years ago

As a side note: I think MvpEventLoop.IsRunning is a candidate for the volatile keyword. But maybe there is a reason it isn't volatile. Personally, I try to wrap those situations in CancellationTokens. Just to be sure.

hudec117 commented 3 years ago

@SebastianBecker2 thank you for the improvements! I didn't have a reason for it not to be marked as volatile so if you believe there's a better way of doing it, I will happily accept it. Let me know if you want to include it in this pull request or another.

SebastianBecker2 commented 3 years ago

I added the volatile keyword. IsRunning is now a full property. With isRunning as the underlying volatile field.

hudec117 commented 3 years ago

Thank you for the fix, I'll look to put out another version onto Nuget in the next week.