1245805 - Content process crash when swapping docShells

Status: RESOLVED DUPLICATE of bug 957691

(Core :: WebRTC, defect, P1)

Description

[Mike de Boer [:mikedeboer]]

In Firefox Hello we now support running chat windows in the content process.
When we detach a chat window from the browser window to a popup, the content process almost immediately crashes.

This is very easy to reproduce by re-enabling and running the 'browser_tearoff.js' mochitest in 'browser/base/content/test/chat/browser.ini'.

I'll attach a crash sig asap.

Comment 1

[Mike de Boer [:mikedeboer]]

[Parent 37351] WARNING: Releasing screensaver: file /Users/mikedeboer/Projects/fx-team/widget/cocoa/nsAppShell.mm, line 83
[Child 37356] WARNING: Slice out of range: 'aStart >= 0 && aEnd <= aSource.mDuration', file /Users/mikedeboer/Projects/fx-team/dom/media/MediaSegment.h, line 355
[Child 37356] WARNING: Audio Buffer is not full by the end of the callback.: 'Available() == 0 || mSampleWriteOffset == 0', file /Users/mikedeboer/Projects/fx-team/dom/media/AudioBufferUtils.h, line 87
[Child 37356] WARNING: Slice out of range: 'aStart >= 0 && aEnd <= aSource.mDuration', file /Users/mikedeboer/Projects/fx-team/dom/media/MediaSegment.h, line 355
Assertion failure: outputTrack->GetEnd() == GraphTimeToStreamTimeWithBlocking(interval.mStart) (Samples missing), at /Users/mikedeboer/Projects/fx-team/dom/media/TrackUnionStream.cpp:268
#01: mozilla::MediaStreamGraphImpl::Process()[/Users/mikedeboer/Projects/fx-team/obj-x86_64-apple-darwin14.5.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x24855f6]
#02: mozilla::MediaStreamGraphImpl::OneIteration(long long)[/Users/mikedeboer/Projects/fx-team/obj-x86_64-apple-darwin14.5.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2485c52]
#03: mozilla::AudioCallbackDriver::DataCallback(float const*, float*, long)[/Users/mikedeboer/Projects/fx-team/obj-x86_64-apple-darwin14.5.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x23c9dac]
#04: audiounit_output_callback[/Users/mikedeboer/Projects/fx-team/obj-x86_64-apple-darwin14.5.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3ad63a5]
#05: AUConverterBase::RenderBus(unsigned int&, AudioTimeStamp const&, unsigned int, unsigned int)[/System/Library/Components/CoreAudio.component/Contents/MacOS/CoreAudio +0x98f5]
#06: AUBase::DoRenderBus(unsigned int&, AudioTimeStamp const&, unsigned int, AUOutputElement*, unsigned int, AudioBufferList&)[/System/Library/Components/CoreAudio.component/Contents/MacOS/CoreAudio +0x6f1e]
#07: AUBase::DoRender(unsigned int&, AudioTimeStamp const&, unsigned int, unsigned int, AudioBufferList&)[/System/Library/Components/CoreAudio.component/Contents/MacOS/CoreAudio +0x560d]
#08: AUHAL::AUIOProc(unsigned int, AudioTimeStamp const*, AudioBufferList const*, AudioTimeStamp const*, AudioBufferList*, AudioTimeStamp const*, void*)[/System/Library/Components/CoreAudio.component/Contents/MacOS/CoreAudio +0xd131]
#09: HALC_ProxyIOContext::IOWorkLoop()[/System/Library/Frameworks/CoreAudio.framework/Versions/A/CoreAudio +0x29f3e]
#10: HALC_ProxyIOContext::IOThreadEntry(void*)[/System/Library/Frameworks/CoreAudio.framework/Versions/A/CoreAudio +0x29612]
#11: HALB_IOThread::Entry(void*)[/System/Library/Frameworks/CoreAudio.framework/Versions/A/CoreAudio +0x294e3]
#12: _pthread_body[/usr/lib/system/libsystem_pthread.dylib +0x405a]
#13: _pthread_body[/usr/lib/system/libsystem_pthread.dylib +0x3fd7]

Comment 2

[Mike de Boer [:mikedeboer]]

Maire, this is quite easy to reproduce on the current fx-team (one the patches in bug 1154277 are merged to m-c, it will there as well):

1. Open NightlyDebug.app and set 'loop.remote.autostart' to 'true' in about:config
2. Start a conversation in Firefox Hello
3. End the conversation and notice that the content process crashed in the active tab.

This might be something your team would like to look at ;-)

Comment 3

[Maire Reavy [:mreavy]]

Hi Mike -- I'm almost certain Pehrsons has a fix for this in his patch queue on Bug 1208371.  It's a huge patch queue (circa 100 patches), but is extremely close to landing. Unfortunately for this bug, Pehrsons just left on PTO for a week.  He's planning to land 1208371 shortly after he gets back if the patches haven't bit-rotted too badly.  I can ask someone to look through Pehrsons' patches to find the fix and see if it can be "cherry-picked" for this bug (no guarantees how quick or successful we'll be, but we can try) -- or we can wait for Pehrsons to get back.  How blocked are you/the team on this bug?

Andreas -- If you can confirm that I'm right about the fix for this being in your patch queue, that would help a lot.  Enjoy your PTO!  And Thanks!

Comment 4

[Mike de Boer [:mikedeboer]]

Holy guacamole, 100!?!? I need to level up, pronto!

It might end up blocking e10s, but we'll see how serious it gets.

Comment 5

[Andreas Pehrson [:pehrsons]]

I recognize the assert at least. Mike, could you see if these two help you somehow:
https://bugzilla.mozilla.org/attachment.cgi?id=8711413
https://bugzilla.mozilla.org/attachment.cgi?id=8711414

Thanks!

Comment 6

[Mike de Boer [:mikedeboer]]

I tried applying the patches and indeed: content process crash is a goner!

(I had to make remove the deprecated isAudio bool arg for StopTrack, which I think you removed in another place as well.)

/me posting the unbitrotted patch here for posterity...

Comment 7

[Mike de Boer [:mikedeboer]]

Attachment: Consolidated crash-fixer patch

Comment 8

[Mike de Boer [:mikedeboer]]

I was curious how our infra would react to these changes: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3893591d2af

Comment 9

[Mike de Boer [:mikedeboer]]

^-- looks to me there are some memory leaks with this patch applied. Other than that, don't see really sad-making stuff...

Comment 10

[Andreas Pehrson [:pehrsons]]

I'd be surprised if this was causing that.

Too many oranges though.. you were referring to the ASAN mochitest one right?

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

I suspect those are all pre-existing ones... (note that they generally match known leaks).  It could be this is making a known leak worse (MediaStreamGraph shutdown issues).  But I think not. We can push a Try without the patch, then retrigger the tests a bunch

Comment 12

[Mike de Boer [:mikedeboer]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d8236abd9bae

Comment 13

[Mike de Boer [:mikedeboer]]

^-- based off m-c, not fx-team. Should be more stable on try too.

Comment 14

[Mike de Boer [:mikedeboer]]

That's looking _lots_ better to me. Randell, could you land the patch for Andreas with a correct commit message? (I've got no clue as to what would be appropriate here :P )
Should the method override that I commented out be removed instead? Or is it fine like this?

Comment 15

[Mike de Boer [:mikedeboer]]

This patch just landed for bug 957691 instead, as https://hg.mozilla.org/integration/mozilla-inbound/rev/1aa33a896ebc