1051230 - Enable leak logging for content processes on desktop

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

It actually works right now, but the content process usually crashes so we never see any leaks.

Comment 1

[Andrew McCreight [:mccr8]]

Thanks, Bill.

Just for some context here, given the plans for a single content process this really needs to work for reals in order to ship e10s.  Without it, we'll have no visibility into severe permanent leaks.  (If we were using process-per-tab it wouldn't be as big of a deal, because content process leaks would only last until somebody closed a tab, but as it is, these leaks are just as bad as pre-e10s.)

Comment 2

[Andrew McCreight [:mccr8]]

Apparently bug 1035454 is enough that we start getting child process leak checking.  But there are a bunch of leaks, so bug 1052224 is going to disable the leak checking.

Comment 3

[Andrew McCreight [:mccr8]]

try run with leak checking enabled: https://tbpl.mozilla.org/?tree=Try&rev=32bf41cea397
(ignore the M3 asserts)

Comment 4

[Andrew McCreight [:mccr8]]

Leak logging works once bug 1035454 is landed, so the bigger issue is fixing the content process leaks, so I'm changing this to be about letting us re-enable it.  The actual enabling (once bug 1035454 is landed) is basically just going to be backing out bug 1052224.

Comment 5

[Andrew McCreight [:mccr8]]

I guess I'm working on this the most, so I'll take the bug.

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Does the testharness automatically do sane things with multiple logs?

Comment 7

[Andrew McCreight [:mccr8]]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #6)
> Does the testharness automatically do sane things with multiple logs?

Yeah, it seems like it.  Check out M2 here:
  https://tbpl.mozilla.org/?tree=Try&rev=32bf41cea397

Comment 8

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #6)
> Does the testharness automatically do sane things with multiple logs?

It seems to:
http://mxr.mozilla.org/mozilla-central/source/build/automationutils.py#376

Comment 9

[Andrew McCreight [:mccr8]]

Attachment: Produce an error for larger child process leaks. WIP

This is a hacky work in progress, but it is useful when locally investigating leaks.  This patch makes it so that we error when a content child leaks, but only if it is larger than 5kb, which is what we always leak.

Comment 10

[Andrew McCreight [:mccr8]]

On top of my hacky pile of patches, e10s M4 has about 620kb of leaks and M5 has about 200kb.  Across a few runs, the size of the former is pretty stables, and the latter was identical in all runs, which is a good sign.
  https://tbpl.mozilla.org/?tree=Try&rev=6ce2c7636985

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Does the large one include JS?  My experience with b2g was that once we got rid of the JS we were leaking the # was extremely stable.

Comment 12

[Andrew McCreight [:mccr8]]

Well, it is only varying by a few hundred bytes, so that's pretty stable.

M2 and M3 hit asserts I added because they are running the GMPService in a content process, so I'm not sure how bad those are going to be once I fix that.  I haven't looked at M1 yet.

Comment 13

[Andrew McCreight [:mccr8]]

B2G has a number of special issues of its own, so I'm splitting that out into a separate bug, bug 1070068.

Comment 14

[Andrew McCreight [:mccr8]]

Comment on attachment 8483012 [details] [diff] [review]
Produce an error for larger child process leaks. WIP

Leak thresholds can now be configured by process type by changing options.leakThresholds, and what processes we error on when they don't produce a log can be adjusted by changing options.ignoreMissingLeaks.

Comment 15

[Andrew McCreight [:mccr8]]

e10s M3 has gone from leaking about 11kb in the tab process to leaking 942kb.

Comment 16

[Andrew McCreight [:mccr8]]

Good: https://hg.mozilla.org/try/rev/d7ec36a77534
Bad: https://hg.mozilla.org/try/rev/388e101e75c8

So that's only a window of a few days.  I'll try to narrow it down to a test and then a regressing changeset at some point.  I tried running the tests that are leaking URLs, but nothing useful came up.

leaking try run: https://tbpl.mozilla.org/?tree=Try&rev=5f123e75dc9c
(I could have also somehow caused the leaks with the patches I added in the try push.)

Comment 17

[Andrew McCreight [:mccr8]]

Bug 1057908 should make us not run the GeckoMediaPluginService in the content process, where it doesn't shut down properly, and thus leaks.

Comment 18

[Jim Mathies [:jimm]]

requesting retriage on this tracker.

Comment 19

[Andrew McCreight [:mccr8]]

(In reply to Jim Mathies [:jimm] from comment #18)
> requesting retriage on this tracker.

This has sort of degenerated into a tracking bug, so I'm not sure it needs to be tracked or not.  We're running XPCOM leak checking on Linux.  Last I checked, it was working on OSX, but we're not actually running it.  Windows has a known issue that needs to be fixed, but that work is being tracked in its own bug, bug 1091917.

Comment 20

[Andrew McCreight [:mccr8]]

(We're not running leak checking on OSX, because it only runs in debug builds, and we're not running e10s debug OSX builds on TreeHerder.)

Comment 21

[Andrew McCreight [:mccr8]]

We're doing this testing now, so no need to leave it as a P1.

Comment 22

[Andrew McCreight [:mccr8]]

This has been done for a while, so I'll mark it as fixed.