Loading the testcase usually triggers one of: Assertion failure: PR_FALSE == lock->locked, at nsprpub/pr/src/pthreads/ptsynch.c:190 Assertion failure: 0 == rv, at nsprpub/pr/src/pthreads/ptsynch.c:360 I hit this frequently in my testing. My opt build crashes less frequently than my debug build asserts, but the crashes looks scary (e.g. some event dispatch code ends up dereferencing the invalid address c0000067).

This error is a result of a recent bug fix that changed the lifetime of nsChannelReader to be owned by an nsAutoPtr. See bug 462878 comment 7. The lifetime of the reader is controlled by liboggplay. liboggplay is calling free on the channel reader when the ogg headers fail to load (due it not being a valid ogg file). We then later call free ourselves due to the autopointer. We either need to request liboggplay maintainers to allow us to maintain ownership of the object ourselves, or revert that change from that bug and fix the issue in that bug another way.

On the other hand, maybe not. liboggplay appears to be doing something else. Discussing with the liboggplay maintaners...

This seems to be resulting in lots of different assertions and crashes, which gets in my way :(

Ok, this is fun. Seems like shutting down the previous mDecodeThread may cause a new load to start. The code really can't handle that, and crashes when non-null mReader gets a new value. I may have a patch soon, but not taking this bug quite yet.

Keep things alive long enough but still allow nsOggDecodeStateMachine to proceed to dispatch NS_NEW_RUNNABLE_METHOD(nsOggDecoder, mDecoder, PlaybackEnded), which may end up to dispatch a DOM event "ended".

Comment on attachment 352583 [details] [diff] [review] possible patch >+ decodeStateMachine = nsnull; >+ reader = nsnull; Note, I kept this on purpose so that the order of destruction doesn't change.

(In reply to comment #4) > Ok, this is fun. Seems like shutting down the previous mDecodeThread may > cause a new load to start. Yeah, the main thread will run other events while it's waiting for the decode thread to terminate in Shutdown(), including JS events, which can do anything, like starting a new load... Bug 468721's patch does the same thing as this one, except it uses an event to shut down the decode thread at a later time. I think that is important, because when we call Stop() from PlaybackEnded(), we don't want to run any JS before we post the ended event (by calling mElement->PlaybackEnded()) further down in nsOggDecoder::PlaybackEnded(). Without this, some JS timeout which starts a new load could fire in Stop(), and we'd then send an ended event to the media element right after that new load was started.

Indeed bug 468721 has very similar patch. I guess it fixes this one too, so my patch isn't needed.

Bug 468721 is fixed1.9.1, so marking this one too.

Verified fixed on trunk and 1.9.1 using builds on OS X and Windows: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090204 Minefield/3.2a1pre ID:20090204020339 Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090204 Shiretoko/3.1b3pre ID:20090204020327

Crashtest: http://hg.mozilla.org/mozilla-central/rev/c38d6ec28c45