Closed Bug 1158404 Opened 5 years ago Closed 5 years ago

[e10s] Leak of DataChannelShutdown

Categories

(Core :: WebRTC: Networking, defect, P4)

defect

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
e10s + ---
firefox40 --- affected
firefox44 --- fixed
Blocking Flags:

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

Attachments

(3 files)

When you run this:

./mach mochitest-plain --e10s --auto-close dom/media/tests/mochitest/test_dataChannel_basicDataOnly.html

The leak includes 1 DataChannelShutdown.  (The rest of the stuff in there is normal for e10s).
tracking-e10s: --- → +
backlog: --- → webRTC+
Rank: 49
Priority: -- → P4
This still happens, though you have to run
  ./mach mochitest -f plain --e10s dom/media/tests/mochitest/test_dataChannel_basicDataOnly.html
and then manually close it
Component: WebRTC → WebRTC: Networking
Another way to trigger this leak is with
  ./mach mochitest -f plain dom/media/tests/mochitest/ipc/test_ipc.html
It looks like this is supposed to be deleted in DataChannelShutdown::Observe(), but it is trying to observe profile-change-net-teardown, which, if I recall correctly, does not happen in the child process.
Does it needs to listen to "content-child-shutdown" instead?

I wonder how much code still adds observers that never fire due to the arrival of e10s. It'd be interesting to instrument addObserver to warn/assert every time an observer is added in the wrong process.
As far as I can tell, there's no need for the global: the observer service holds a strong reference and nobody else looks at the global, except to clear it in shutdown. This fixes the leak, but of course does not fix the issue of this code never running.
(In reply to Jan-Ivar Bruaroey [:jib] from comment #4)
> I wonder how much code still adds observers that never fire due to the
> arrival of e10s. It'd be interesting to instrument addObserver to
> warn/assert every time an observer is added in the wrong process.

That's a good idea! I filed bug 1216252.
Assignee: nobody → continuation
Blocks: 1215148
Attachment #8676374 - Flags: review?(rjesup) → review+
Attachment #8676376 - Flags: review?(rjesup) → review+
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.