Closed Bug 1574965 Opened 5 months ago Closed 5 months ago

Control MediaStreamGraph shutdown from the main thread

Categories

(Core :: Audio/Video: MediaStreamGraph, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: pehrsons, Assigned: pehrsons)

References

(Blocks 1 open bug)

Details

Crash Data

Attachments

(2 files)

In light of recent MediaStreamGraph lifetime issues (see See Also) we could simplifiy things by deciding on main thread in advance when a graph will be shutdown.

We can do this by keeping a MediaStream count that's incremented in AddStream() and decremented in MediaStream::Destroy(). When this reaches zero after having been non-zero we remove the graph from gGraphs. Like proposed in bug 1572858 comment 3 we should assert in AddStream() that the current graph is present in gGraphs to catch invalid uses.

Possibly we need a tail-dispatched runnable to take the decision to shut down, such that an event loop task doing stream->Destroy(); graph->AddStream(stream2); doesn't shut the graph down. Perhaps an audit of existing uses of both Destroy and AddStream could shine some light on the need for this.

Remaining lifetime logic can remain in place, so that the graph shuts down automatically when things have been cleaned up on the graph thread.

This effectively also removes the use case and need for GraphDriver::Revive, allowing us to do a bunch of bonus cleanup in both MediaStreamGraph and GraphDriver.

Priority: -- → P3
Assignee: nobody → apehrson
Status: NEW → ASSIGNED

This gives control of the graph's lifetime to main thread. Not being in gGraphs
means it's not meant to be used for any new streams as it will irreversibly be
shutting down.

This obsoletes GraphDriver::Revive.

Depends on D43596

Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/77e0581c780d
Remove a graph that becomes empty from gGraphs synchronously. r=karlt,padenot
https://hg.mozilla.org/integration/autoland/rev/40dc5087be4d
Remove GraphDriver::Revive. r=karlt,padenot
Regressions: 1577534
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
No longer blocks: 1576662
Duplicate of this bug: 1576662
Crash Signature: [@ mozilla::AudioCallbackDriver::Revive()]
See Also: → 1571165
You need to log in before you can comment on or make changes to this bug.