If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

nsStreamStrategy::~nsStreamStrategy triggers ptsynch.c lock assertions

VERIFIED FIXED in mozilla1.9.2a1

Status

()

Core
Audio/Video
P2
critical
VERIFIED FIXED
9 years ago
8 years ago

People

(Reporter: Jesse Ruderman, Unassigned)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla1.9.2a1
assertion, crash, testcase, verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
wanted1.9.0.x -
wanted1.8.1.x -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?])

Attachments

(2 attachments)

(Reporter)

Description

9 years ago
Created attachment 350290 [details]
testcase (can crash Firefox when loaded)

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).
Flags: blocking1.9.1?
(Reporter)

Updated

9 years ago
Whiteboard: [sg:critical?]
Flags: blocking1.9.1? → blocking1.9.1+

Comment 1

9 years ago
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.

Comment 2

9 years ago
On the other hand, maybe not. liboggplay appears to be doing something else. Discussing with the liboggplay maintaners...
Priority: -- → P2
(Reporter)

Comment 3

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

Comment 4

9 years ago
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.

Comment 5

9 years ago
Created attachment 352583 [details] [diff] [review]
possible patch

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".
Assignee: nobody → Olli.Pettay
Status: NEW → ASSIGNED
Attachment #352583 - Flags: review?(chris.double)

Comment 6

9 years ago
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.

Comment 8

9 years ago
Indeed bug 468721 has very similar patch. I guess it fixes this one too, so my
patch isn't needed.
Depends on: 468721

Updated

9 years ago
Attachment #352583 - Flags: review?(chris.double)

Updated

9 years ago
Assignee: Olli.Pettay → nobody
(Reporter)

Updated

9 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Bug 468721 is fixed1.9.1, so marking this one too.
Keywords: fixed1.9.1
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
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
Keywords: fixed1.9.1 → verified1.9.1
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → mozilla1.9.2a1
Flags: wanted1.9.0.x-
Flags: wanted1.8.1.x-
Group: core-security
(Reporter)

Comment 11

8 years ago
Crashtest: http://hg.mozilla.org/mozilla-central/rev/c38d6ec28c45
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.