Closed Bug 1254298 Opened 9 years ago Closed 9 years ago

Platform specific gamepad services can leak on shutdown in e10s

Categories

(Core :: DOM: Device Interfaces, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
e10s + ---
firefox47 --- affected
firefox48 --- fixed

People

(Reporter: qdot, Assigned: qdot)

References

Details

(Whiteboard: btpp-active)

Attachments

(1 file)

In the process of landing bug 1156957, a leak of nsTArray_base was found on shutdown. This resolved back to the mGamepads member of LinuxGamepadService, meaning that the Service was still alive at time of shutdown, which it shouldn't be. The logs show that the GamepadListenerRemoved IPC message is sent after the shutdown of the child actor, meaning the parent never receives the message, and therefore never shuts down the platform service. We should make sure that gamepad platform services are cleanly shut down both when all listeners are removed, and on XPCOM shutdown.
I think this leak can be reproduced under the following process. 1. Open firefox nightly (I use ./mach run) 2. Use the FIRST tab to open https://html5gamepad.com and plug a gamepad in 3. Open a new tab but don't do anything about it 4. Close the first tab (The gamepad is still plugged in), waiting for 5 seconds 5. Go to https://html5gamepad.com with the Tab created in 3. You will find that this website cannot detect gamepad unless you reconnect it. It is because gamepad service doesn't shutdown after the first tab is closed, the gamepad service thinks your gamepad is always attached so it doesn't fire GamepadAdded event to the new tab. This problem only exists in Nightly build under Linux and Mac OSX, Windows is not affected. Interestingly, only the first tab has this issue, So I think it is something goes wrong when firefox startup.
I think this may have something to do with the Cleanup Timer in GamepadService. Whenever we lose our last nsGlobalWindow listener, a 2 second timer is started to clean up the service. This is ok behavior in-process, but out of process this means that we can try to clean up after the child has closed. I bet this is leading to other problems too, like the one mentioned in Comment 1. Would like to find a minimum viable fix for this, so we can concentrate on the PBackground implementation. I think it might just be shutting down directly in e10s situations, and only using the timer on non-e10s? Will give that a shot.
Ok, since it seems to work locally, here's the try for aforementioned mfv (don't run cleanup timer on e10s): https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ca2ae89c7bc
This patch makes GamepadService run the IPC cleanup code directly on the last listener detach in the e10s case. In the in-process case, we still keep the usual Timer solution.
Attachment #8728703 - Flags: review?(ted)
Attachment #8728703 - Flags: review?(cleu)
Comment on attachment 8728703 [details] [diff] [review] Patch 1 (v1) - Bypass GamepadService cleanup timer on e10s Review of attachment 8728703 [details] [diff] [review]: ----------------------------------------------------------------- I see no potential hazard to remove cleanup timer under e10s. So I think this patch is OK.
Attachment #8728703 - Flags: review?(cleu) → review+
tracking-e10s: --- → +
Priority: -- → P3
Comment on attachment 8728703 [details] [diff] [review] Patch 1 (v1) - Bypass GamepadService cleanup timer on e10s Review of attachment 8728703 [details] [diff] [review]: ----------------------------------------------------------------- This seems pretty sensible.
Attachment #8728703 - Flags: review?(ted) → review+
Whiteboard: btpp-active
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: