1323832 - Leaks not being reported when chaining MozPromise

Status: RESOLVED WORKSFORME

(Core :: Audio/Video: Playback, defect, P1)

Description

[Jean-Yves Avenard [:jya]]

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

Comment 1

[Jean-Yves Avenard [:jya]]

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

Comment 2

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Jean-Yves - is this bug still an issue?

Comment 3

[Jean-Yves Avenard [:jya]]

The code is gone, promise chaining has changed, can't verify if that would still be the case or not.