523335 - hang on shutdown involving ogg video

Status: RESOLVED FIXED

(Core :: Audio/Video, defect, P2)

Description

[(not currently active) Ted Mielczarek]

Attachment: apple crashreporter sample

I tried to restart my Minefield to apply a nightly update, and it hung. I "Force Quit" it and clicked "Report" on the Apple dialog, which gave me a nice little sample of process activity (attached). The main thread is spinning the event loop, and there's an oggplayer thread doing something, but I can't really tell what's going on there. I currently have a 20091019 build, so I suspect this was the 20091018 nightly.

I didn't currently have any ogg video loaded, but I had watched Air Mozilla yesterday and hadn't restarted my browser since then. (I had closed the tab, however.)

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

STR: (Not always reproducible)

1. Load Air Mozilla.
2. Start playing a previous video.
3. Seek somewhere unbuffered.
4. Before seek has completed, navigate away to another page.
5. Close Firefox. Should hang on shutdown.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This definitely blocks.

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to comment #1)
> STR: (Not always reproducible)

Actually we don't need to navigate to another page in STR Step 4, we can still hang if we shutdown while a seek is in progress. Nasty.

Comment 4

[(not currently active) Ted Mielczarek]

FWIW, I don't recall doing any explicit seeks, since I was watching live streams. The streams did stop on me a few times, and I reloaded or clicked play, so maybe that triggers the same response somehow.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

Looks like we're not checking for underlying stream close in oggz_bounded_seek_set() when we call oggz_seek_raw, and our bisection loop is looping infinitely.

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch v1

Patch with testcase that will cause hangs when mochitest tries to shutdown.

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch v2

Oops, I added the test file to the ogg videos list, not the ogg tests list in the Makefile.am. It still worked, but was messy. This patch fixes that, and renames the test to use "_" instead of "-" in its filename, to match the others... Otherwise it's the same as previous.

Comment 8

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch v3

Patch with backend-independent mochitest.

Comment 9

[Chris Pearce [:cpearce (Not reading bugmail)]]

http://hg.mozilla.org/mozilla-central/rev/9d4bef551670

Comment 10

[Matthew Gregan [:kinetik]]

Do we want this on 1.9.1 as well?  It looks like the code is the same there.

Comment 11

[Matthew Gregan [:kinetik]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/1c35e7857f4a