1189698 - Race condition shutting down the old player thread

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[[e-mail]]

Attachment: Stack backtrace

I have an add-on that invokes the nsISound API at certain times, however as of late it has become very crashy. Looking at the stack frames, I notice that the thread that nsSound is trying to shut down is already nullptr in the nsSound instance, and the value of "this" in the next lower frame points to deleted memory, so what I assume happened is that the sound releaser got dispatched while the main thread was trying to shut down the player thread. This would have caused the call to shutdown to fail, but the sound code doesn't check that.

Comment 1

[[e-mail]]

Attachment: Proposed patch

By moving the value of mPlayerThread into a local the function becomes reentrant.

Comment 2

[Jim Mathies [:jimm]]

Comment on attachment 8641653 [details] [diff] [review]
Proposed patch

Review of attachment 8641653 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/nsSound.cpp
@@ +124,5 @@
>  
>  void nsSound::ShutdownOldPlayerThread()
>  {
> +  nsCOMPtr<nsIThread> playerThread(mPlayerThread.forget());
> +  if (playerThread)

nit - brace me

Comment 3

[[e-mail]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a48677d48570

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a48677d48570