1062443 - Running ~AudioChannelAgent during shutdown in a child process crashes

Status: RESOLVED WONTFIX

(Core :: DOM: Core & HTML, defect)

Description

[Andrew McCreight [:mccr8]]

I'm not sure what component this belongs in.  Maybe web audio?  This shows up when running the test
  dom/browser-element/mochitest/priority/test_Audio.html

It looks like this creates an OOP browser element, then it plays a sound in it.  We start shutting down the content process, and destroy AudioChannelAgent, which causes it to try to send a message to the parent, but I guess we're too late in shutdown to actually send a message.  This causes the content process to abort, so we can't check the leak log.

Here's an excerpt from the log where this is failing, where I modified ContentChild::QuickExit() to do an NS_ASSERTIO(false) instead of a warning.

148 INFO --DOMWINDOW == 0 (0x7f67b33ad000) [pid = 21122] [serial = 1] [outer = (nil)] [url = http://mochi.test:8888/tests/dom/browser-element/mochitest/priority/file_Audio.html]
149 INFO ###!!! [Child][MessageChannel::SendAndWait] Error: Closed channel: cannot send/recv
150 INFO [Child 21122] ###!!! ASSERTION: content process _exit()ing: 'false', file /home/amccreight/mc/dom/ipc/ContentChild.cpp, line 1500
151 INFO mozilla::dom::ContentChild::ProcessingError(mozilla::ipc::HasResultCodes::Result) (/home/amccreight/mc/dom/ipc/ContentChild.cpp:1477)
152 INFO mozilla::Monitor::Lock() (/home/amccreight/mc/obj-dbg/ipc/glue/../../dist/include/mozilla/Monitor.h:35)
153 INFO ~nsAutoPtr (/home/amccreight/mc/obj-dbg/ipc/glue/../../dist/include/nsAutoPtr.h:73)
154 INFO mozilla::ipc::MessageChannel::Send(IPC::Message*, IPC::Message*) (/home/amccreight/mc/ipc/glue/MessageChannel.cpp:630)
155 INFO mozilla::dom::PContentChild::SendAudioChannelUnregisterType(mozilla::dom::AudioChannel const&, bool const&, bool const&) (/tmp/./PContentChild.cpp:2488)
156 INFO mozilla::dom::AudioChannelServiceChild::UnregisterAudioChannelAgent(mozilla::dom::AudioChannelAgent*) (/home/amccreight/mc/dom/audiochannel/AudioChannelServiceChild.cpp:119)
157 INFO mozilla::dom::AudioChannelAgent::StopPlaying() (/home/amccreight/mc/dom/audiochannel/AudioChannelAgent.cpp:140)
158 INFO ~AudioChannelAgent (/home/amccreight/mc/dom/audiochannel/AudioChannelAgent.cpp:37)
159 INFO operator delete(void*) (/tmp/../../dist/include/mozilla/mozalloc.h:225)
160 INFO ~SnowWhiteKiller (/home/amccreight/mc/xpcom/base/nsCycleCollector.cpp:2618)
161 INFO nsCycleCollector::FreeSnowWhite(bool) (/home/amccreight/mc/xpcom/base/nsCycleCollector.cpp:2797)
162 INFO nsCOMPtr<nsICycleCollectorListener>::get() const (/tmp/../../dist/include/nsCOMPtr.h:816)
163 INFO nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*) (/home/amccreight/mc/xpcom/base/nsCycleCollector.cpp:3583)
164 INFO nsCycleCollector::ShutdownCollect() (/home/amccreight/mc/xpcom/base/nsCycleCollector.cpp:3536)
165 INFO nsRefPtr<nsCycleCollector>::assign_assuming_AddRef(nsCycleCollector*) (/tmp/../../dist/include/nsAutoPtr.h:836)
166 INFO mozilla::ShutdownXPCOM(nsIServiceManager*) (/home/amccreight/mc/xpcom/build/nsXPComInit.cpp:934)
167 INFO XRE_TermEmbedding (/home/amccreight/mc/toolkit/xre/nsEmbedFunctions.cpp:197)
168 INFO mozilla::ipc::ScopedXREEmbed::Stop() (/home/amccreight/mc/ipc/glue/ScopedXREEmbed.cpp:111)

Comment 1

[Andrew McCreight [:mccr8]]

Is this AudioChannelAgent always going to live as long as the process once it is started, or will it sometimes exit earlier?  In the former case, it seems like this unregister message is pointless.  In the latter case, I can just tell it to not send the message when we're shutting down.

Comment 2

[Andrea Marchesini [:baku]]

AudioChannelAgent is destroyed when a MediaElement stops playing. It doesn't live as long as the process does. We need to find a different solution.

Comment 3

[Andrea Marchesini [:baku]]

mccr8, I think the best solution is:

AudioChannelAgent should be an nsIObserver and listen for: xpcom-shutdown. In the DTOR, we should not send a message to IPC if we are in shutdown. I'm happy to write a patch.

Comment 4

[Andrew McCreight [:mccr8]]

(In reply to Andrea Marchesini (:baku) from comment #3)
> AudioChannelAgent should be an nsIObserver and listen for: xpcom-shutdown.
> In the DTOR, we should not send a message to IPC if we are in shutdown. I'm
> happy to write a patch.

That would be great, thanks.

Comment 5

[Andrea Marchesini [:baku]]

Attachment: audio.patch

The patch is this but I was not able to test it in any way. Do you have some idea? Closing the iframe where an e10s process is running is not enough.

Comment 6

[Andrew McCreight [:mccr8]]

dom/browser-element/mochitest/priority/test_Audio.html is where I was having this problem, but it requires some other patches so we don't crash for other reasons during shutdown, so I can test it.

Comment 7

[Andrew McCreight [:mccr8]]

Comment on attachment 8493811 [details] [diff] [review]
audio.patch

This seems to make dom/base/test/test_audioNotification.html time out, even without any other patches applied.  (I'm about to check what this does to the shutdown failure I filed the bug about, but I thought I'd mention that.)

See the M1 failure here:
  https://tbpl.mozilla.org/?tree=Try&rev=294d622e66c9

I confirmed it locally without any other patches.

Comment 8

[Andrew McCreight [:mccr8]]

Other than that test failure, it does fix the late message sending. :)

To see that, apply the patch from bug 1035454, then change the MsgDropped case in ContentChild::ProcessingError() from doing a QuickExit() to doing NS_ASSERTION(false, "Dropping message!"); return;

Then run dom/browser-element/mochitest/priority/test_Audio.html, which is only run on Linux.  Without your patch, it hits the assertion.  With your patch, it does not.

Comment 9

[Andrew McCreight [:mccr8]]

It looks like this has a bc2 failure, too.  Or at least, I'm guessing this is at fault. :)
  https://tbpl.mozilla.org/?tree=Try&rev=a036840fcee6

Comment 10

[Andrew McCreight [:mccr8]]

Thanks to bug 1080017, there is now a method ContentChild::IsAlive() which returns false after ActorDestroy has been called.  Maybe that will simplify this.

Comment 11

[Andrew McCreight [:mccr8]]

Attachment: AudioChannelServiceChild should only send a message to the parent if ContentChild is alive.

I'm not quite sure if I want to need to land this now, but it also fixes the test_Audio crash.

Comment 12

[Andrea Marchesini [:baku]]

This bug is going to be fixed by bug 1113086