Closed
Bug 1323832
Opened 8 years ago
Closed 7 years ago
Leaks not being reported when chaining MozPromise
Categories
(Core :: Audio/Video: Playback, defect, P1)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: jya, Unassigned)
References
Details
This is an issue I've been struggling trying to get bug 1319992 landed.
In bug 1319992, we have an object DemuxerProxy allocated on the heap. When that object is destructed, it queues a task on a given task queue that will release the reference to a refcounted object (MediaDataDemuxer).
~DemuxerProxy()
{
MOZ_COUNT_DTOR(DemuxerProxy);
RefPtr<Data> data = mData.forget();
mTaskQueue->Dispatch(
// We need to clear our reference to the demuxer now. So that in the
// event the init promise wasn't resolved, such as what can happen with
// the mediasource demuxer that is waiting on more data, it will force
// the init promise to be rejected.
NS_NewRunnableFunction([data]() {
data->mDemuxer = nullptr;
}));
}
The MediaDataDemuxer object could currently be in used in a chained promise like so:
https://hg.mozilla.org/try/file/267ffb797225/dom/media/MediaFormatReader.cpp#l699
That is it does:
ASSERT(OnTaskQueue1); return InvokeAsync(TaskQueue2, []() { return MozPromise<T>(); } ) -> Then(TaskQueue2, []() { ... });
This will cause leaks:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=267ffb79722576c291df463b8ec5b4a8270c796a
It shows that the mDemuxer object is never released, the task to release the object didn't run, the captured refcounted object data wasn't released.
If however, we change the way the MozPromise are chained from:
ASSERT(OnTaskQueue1); return InvokeAsync(TaskQueue2, []() { return MozPromise<T>(); } ) -> Then(TaskQueue2, []() { ... });
to:
ASSERT(OnTaskQueue1); return InvokeAsync(TaskQueue2, []() { return MozPromise<T>() -> Then(TaskQueue2, []() { ... }); } );
That is the lambdas are nested instead, having them all run on the same taskqueue then we get no leak:
code change: https://hg.mozilla.org/try/rev/d389c2566dd633bfd9d845a891cb4d08ab5452fa
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d389c2566dd633bfd9d845a891cb4d08ab5452fa
Out of 15 runs, not a single leak occurred. While with the MozPromise chained rather than nested, we get 100% failure.
Something fishy is happening.
Lodging this bug so it can be investigated further
Reporter | ||
Updated•8 years ago
|
Flags: needinfo?(jwwang)
Reporter | ||
Comment 1•8 years ago
|
||
Ok...
so there was a leak, occurs in the webaudio mochitest when using a video file as source.
The MediaFormatReader never retrieved the video demuxer, and as such, never calls BreakCycle on it.
Now why would it be that the leaks is reported by ASAN when chaining the MozPromise, but not when you nest them within InvokeAsync
Reporter | ||
Updated•8 years ago
|
Summary: Leaks occurring when chaining MozPromise and/or task not being dispatched. → Leaks not being reported when chaining MozPromise
Updated•8 years ago
|
Priority: -- → P1
Jean-Yves - is this bug still an issue?
Flags: needinfo?(jyavenard)
Reporter | ||
Comment 3•7 years ago
|
||
The code is gone, promise chaining has changed, can't verify if that would still be the case or not.
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(jyavenard)
Resolution: --- → WORKSFORME
Reporter | ||
Updated•7 years ago
|
Flags: needinfo?(jwwang)
You need to log in
before you can comment on or make changes to this bug.
Description
•