If the testcase doesn't trigger at the first try, refresh the page one or two times. alloc: ./content/media/webaudio/AudioNode.cpp:174 void AudioNode::Connect(AudioNode& aDestination, uint32_t aOutput, uint32_t aInput, ErrorResult& aRv) { [...] * ps->AllocateInputPort(mStream, MediaInputPort::FLAG_BLOCK_INPUT, static_cast<uint16_t>(aInput), static_cast<uint16_t>(aOutput)); [...] free: ./content/media/MediaStreamGraph.cpp:1882 void MediaInputPort::Destroy() { class Message : public ControlMessage { public: Message(MediaInputPort* aPort) : ControlMessage(nullptr), mPort(aPort) {} virtual void Run() { mPort->Disconnect(); --mPort->GraphImpl()->mPortCount; * NS_RELEASE(mPort); } [...] } re-use: ./content/media/MediaStreamGraph.h:737 * NS_INLINE_DECL_THREADSAFE_REFCOUNTING(MediaInputPort) Tested with m-i changeset: 132784:57f555f96ac9 and the patch in bug 874952

Triaged branch flags with Ehsan.

OK, so MediaInputPort::Destroy::Command holds a plain pointer to MediaInputPort, and NS_RELEASES it directly, claiming that the graph is the last thing holding on to the MediaInputPort. That is completely incorrect. You can allocate an input port and hold on to it as much as you want.

And FWIW, this bug should have been discovered before by fuzzing WebRTC etc, I would have expected.

I doubt the sequence that causes this is possible in webrtc, which only uses MediaInputPorts as part of nsDOMUserMediaStream in MediaManager.cpp (aka getUserMedia). The failure paths there are minimal, and it's closely tied to the associated MediaStream/TrackUnion.

After fixing that part of the bug, I hit bug 875221 on this test case.

(In reply to Randell Jesup [:jesup] from comment #5) > I doubt the sequence that causes this is possible in webrtc, which only uses > MediaInputPorts as part of nsDOMUserMediaStream in MediaManager.cpp (aka > getUserMedia). The failure paths there are minimal, and it's closely tied > to the associated MediaStream/TrackUnion. Hmm, actually, maybe this is only an issue for the non-realtime MSGs...

Comment on attachment 753493 [details] [diff] [review] Patch (v1) Review of attachment 753493 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/MediaStreamGraph.cpp @@ +1906,5 @@ > virtual void RunDuringShutdown() > { > Run(); > } > + nsRefPtr<MediaInputPort> mPort; This is going to leak since you're adding a reference we weren't adding before. The comment is definitely wrong, but I'm not sure why the NS_RELEASE is wrong. We need to release the MediaStreamGraph's reference to the port.

Hmm, I think you're right. For some reason the leak detector doesn't detect that. :( So perhaps the real problem here is that mConsumers holds weak pointers to MediaInputPorts, and everything else holds a strong reference to it. If we made those nsRefPtrs, we should be able to remove this NS_RELEASE and just make the ControlCommand hold a normal nsRefPtr. Does that make sense?

Read the comment on MediaInputPort in MediaStreamGraph.h :-) Note that in ProcessedMediaStream::AllocateInputPort, in the Run method that runs in the MediaStreamGraph thread, we have // The graph holds its reference implicitly mPort.forget(); So the already_AddRefed we throw away there is supposed to be released by the NS_RELEASE we're doing in Destroy's Run method. There shouldn't be a need to use nsRefPtrs actually inside the MediaStreamGraph. So, I don't know why this code isn't working.

OK, so the problem here is that if ProcessedMediaStream::AllocateInputPort::Command is run during shutdown, it Release()'s mPort without registering it with the graph, and then later on we NS_RELEASE the pointer because we think that the graph is holding a reference to it, while it's not.

With the patch v2, we seem to hit another bug with this callstack...

Christoph, can you please see if you can minimize the test case once more with this patch applied?

:Ehsan, I have noticed something similar, see: https://bugzilla.mozilla.org/show_bug.cgi?id=875221#c6 Yes, give me a few minutes.

(In reply to Christoph Diehl [:cdiehl] from comment #18) > Created attachment 753840 [details] > testcase-reduced-after-patch-2 Attached this to bug 875221.

Backed out, because the crashtest leaks: https://hg.mozilla.org/integration/mozilla-inbound/rev/c4a85f341ffd https://tbpl.mozilla.org/php/getParsedLog.php?id=23371046&tree=Cypress

The leak is bug 875911.

Landed with the fix to bug 875911 and converted the test to a mochitest to use SpecialPowers instead of fuzzPriv to force GC and CC.

Mass moving Web Audio bugs to the Web Audio component. Filter on duckityduck.

No longer tracking for FF23