1217218 - Intermittent LeakSanitizer | leak at nsPerformanceGroup::Make, nsPerformanceStatsService::nsPerformanceStatsService, nsPerformanceStatsServiceConstructor

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), defect)

Description

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://treeherder.mozilla.org/logviewer.html#?job_id=5251702&repo=fx-team

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Possibly fallout from bug 1208747?

Comment 2

[David Teller [:Yoric] - still alive but not very active]

Most likely, yes.

Comment 3

[David Teller [:Yoric] - still alive but not very active]

I'll wait until we have a few more samples to determine what's going on.

Comment 4

[Andrew McCreight [:mccr8]]

This is happening very frequently, across a number of test suites.

Comment 5

[Andrew McCreight [:mccr8]]

(In reply to Andrew McCreight [:mccr8] from comment #4)
> This is happening very frequently, across a number of test suites.

Well, all ASan. I just meant, in different mochitest chunks.

Comment 6

[Andrew McCreight [:mccr8]]

(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #3)
> I'll wait until we have a few more samples to determine what's going on.

Do you have a fix for this, or should bug 1208747 get backed out? Thanks.

Comment 7

[David Teller [:Yoric] - still alive but not very active]

I still have only a single log to work on, so there are only so many things I can do at this stage. Andrew, is it always a leak of a single PerformanceGroup?

However, based on this log, it looks like a single js::PerformanceGroup is leaking. There are very few structures that can hold references to a js::PerformanceGroup:
- a JS Compartment holds one (released when the compartment is destroyed);
- a JS Runtime holds a few (any leaking reference is released when the JS Runtime is destroyed);
- the nsPerformanceGroupService holds one.

There are also a few references on the stack.

Could it be possible that a single JS Compartment is never garbage-collected?

Could it be possible that we finish the process from a nested event loop which prevents the stack-based reference from being released?

Comment 8

[David Teller [:Yoric] - still alive but not very active]

Oh, I missed some information. I see which reference is not collected, I hope to have a patch shortly.

Comment 9

[Andrew McCreight [:mccr8]]

(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #7)
> I still have only a single log to work on, so there are only so many things
> I can do at this stage.

If you click on the link in the "Orange factor" section in the upper right part of this page, you'll get a list of all of the times this has been marked.

Comment 10

[David Teller [:Yoric] - still alive but not very active]

(In reply to Andrew McCreight [:mccr8] from comment #9)
> If you click on the link in the "Orange factor" section in the upper right
> part of this page, you'll get a list of all of the times this has been
> marked.

I have no such link, either on Bugzilla or in the Treeherder link.

Comment 11

[David Teller [:Yoric] - still alive but not very active]

Attachment: MozReview Request: Bug 1217218 - Consolidating shutdown of nsPerformanceStatsService;r=froydnj

Bug 1217218 - nsPerformanceStatsService now clears the reference to mTopGroup in Dispose();r?froydnj

Comment 12

[Andrew McCreight [:mccr8]]

(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #10)
> I have no such link, either on Bugzilla or in the Treeherder link.

Here's the link: https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1217218

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #10)
> (In reply to Andrew McCreight [:mccr8] from comment #9)
> > If you click on the link in the "Orange factor" section in the upper right
> > part of this page, you'll get a list of all of the times this has been
> > marked.
> 
> I have no such link, either on Bugzilla or in the Treeherder link.

I believe that's not available with the new modal interface. (bug 1153105)

Comment 14

[Intermittent Failures Robot]

50 automation job failures were associated with this bug yesterday.

Repository breakdown:
* mozilla-inbound: 22
* fx-team: 11
* mozilla-central: 8
* b2g-inbound: 8
* try: 1

Platform breakdown:
* linux64: 50

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1217218&startday=2015-10-22&endday=2015-10-22&tree=all

Comment 15

[Nathan Froyd [:froydnj]]

Comment on attachment 8677608 [details]
MozReview Request: Bug 1217218 - Consolidating shutdown of nsPerformanceStatsService;r=froydnj

https://reviewboard.mozilla.org/r/22973/#review20565

Comment 16

[David Teller [:Yoric] - still alive but not very active]

I don't think this is really MemShrink, as solving this bug won't change the memory footprint of Firefox by a single byte.

Comment 17

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8677608 [details]
MozReview Request: Bug 1217218 - Consolidating shutdown of nsPerformanceStatsService;r=froydnj

Bug 1217218 - nsPerformanceStatsService now clears the reference to mTopGroup in Dispose();r=froydnj

Comment 18

[Andrew McCreight [:mccr8]]

(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #16)
> I don't think this is really MemShrink, as solving this bug won't change the
> memory footprint of Firefox by a single byte.

It obviously isn't a major issue, but maintaining the integrity of our memory testing infrastructure is an important part of monitoring Firefox's memory usage.

Comment 19

[Intermittent Failures Robot]

36 automation job failures were associated with this bug yesterday.

Repository breakdown:
* mozilla-inbound: 15
* mozilla-central: 8
* fx-team: 7
* b2g-inbound: 6

Platform breakdown:
* linux64: 36

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1217218&startday=2015-10-23&endday=2015-10-23&tree=all

Comment 20

[Intermittent Failures Robot]

110 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 50
* fx-team: 21
* mozilla-central: 19
* b2g-inbound: 14
* try: 6

Platform breakdown:
* linux64: 110

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1217218&startday=2015-10-19&endday=2015-10-25&tree=all

Comment 21

[David Teller [:Yoric] - still alive but not very active]

I don't know why MozReview => Try doesn't work for me atm.

Comment 22

[David Teller [:Yoric] - still alive but not very active]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=adb0414c2149

Comment 23

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8677608 [details]
MozReview Request: Bug 1217218 - Consolidating shutdown of nsPerformanceStatsService;r=froydnj

Bug 1217218 - Consolidating shutdown of nsPerformanceStatsService;r?froydnj

In e10s tests, the content process version of nsPerformanceStatsService doesn't always shut down correctly. Apparently, this is due to content-child-shutdown not always being received - or possibly just
not being received on time. We fix this by extending shutdown to also shutdown the service in reaction to xpcom-will-shutdown. This raises an additional issue with JS code being able to request a
snapshot after shutdown of the service, so we add a sanity check to ensure that such calls always fail once the service is shutdown.

Comment 24

[Intermittent Failures Robot]

30 automation job failures were associated with this bug yesterday.

Repository breakdown:
* mozilla-inbound: 15
* mozilla-central: 8
* fx-team: 4
* b2g-inbound: 2
* try: 1

Platform breakdown:
* linux64: 30

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1217218&startday=2015-10-26&endday=2015-10-26&tree=all

Comment 25

[David Teller [:Yoric] - still alive but not very active]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=31f3bd205f87

Comment 26

[Intermittent Failures Robot]

34 automation job failures were associated with this bug yesterday.

Repository breakdown:
* mozilla-inbound: 10
* mozilla-central: 8
* fx-team: 8
* b2g-inbound: 7
* try: 1

Platform breakdown:
* linux64: 34

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1217218&startday=2015-10-27&endday=2015-10-27&tree=all

Comment 27

[Intermittent Failures Robot]

35 automation job failures were associated with this bug yesterday.

Repository breakdown:
* mozilla-inbound: 18
* fx-team: 6
* mozilla-central: 5
* b2g-inbound: 4
* try: 2

Platform breakdown:
* linux64: 35

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1217218&startday=2015-10-28&endday=2015-10-28&tree=all

Comment 28

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8677608 [details]
MozReview Request: Bug 1217218 - Consolidating shutdown of nsPerformanceStatsService;r=froydnj

Oh, forgot to re-r? it.

Comment 29

[Nathan Froyd [:froydnj]]

https://reviewboard.mozilla.org/r/22973/#review21155

Comment 30

[Nathan Froyd [:froydnj]]

Comment on attachment 8677608 [details]
MozReview Request: Bug 1217218 - Consolidating shutdown of nsPerformanceStatsService;r=froydnj

Apparently reviewboard got confused because I had already reviewed the patch, or something?

Comment 31

[David Teller [:Yoric] - still alive but not very active]

Yeah, see bug 1219646.

Comment 32

[David Teller [:Yoric] - still alive but not very active]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=31f3bd205f87

Comment 33

[Intermittent Failures Robot]

52 automation job failures were associated with this bug yesterday.

Repository breakdown:
* mozilla-central: 19
* mozilla-inbound: 15
* b2g-inbound: 13
* fx-team: 4
* try: 1

Platform breakdown:
* linux64: 52

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1217218&startday=2015-10-29&endday=2015-10-29&tree=all

Comment 34

[Carsten Book [:Tomcat]]

this failed to apply :

patching file toolkit/components/perfmonitoring/nsPerformanceStats.cpp
Hunk #4 FAILED at 501
Hunk #5 FAILED at 641
2 out of 5 hunks FAILED -- saving rejects to file toolkit/components/perfmonitoring/nsPerformanceStats.cpp.rej
patch failed to apply


could you rebase this too ?

Comment 35

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8677608 [details]
MozReview Request: Bug 1217218 - Consolidating shutdown of nsPerformanceStatsService;r=froydnj

Bug 1217218 - Consolidating shutdown of nsPerformanceStatsService;r?froydnj

In e10s tests, the content process version of nsPerformanceStatsService doesn't always shut down correctly. Apparently, this is due to content-child-shutdown not always being received - or possibly just
not being received on time. We fix this by extending shutdown to also shutdown the service in reaction to xpcom-will-shutdown. This raises an additional issue with JS code being able to request a
snapshot after shutdown of the service, so we add a sanity check to ensure that such calls always fail once the service is shutdown.

Comment 36

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8677608 [details]
MozReview Request: Bug 1217218 - Consolidating shutdown of nsPerformanceStatsService;r=froydnj

Bug 1217218 - Consolidating shutdown of nsPerformanceStatsService;r=froydnj

Comment 37

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8677608 [details]
MozReview Request: Bug 1217218 - Consolidating shutdown of nsPerformanceStatsService;r=froydnj

Bug 1217218 - Consolidating shutdown of nsPerformanceStatsService;r=froydnj

Comment 38

[David Teller [:Yoric] - still alive but not very active]

Rebased.

Comment 39

[Intermittent Failures Robot]

55 automation job failures were associated with this bug yesterday.

Repository breakdown:
* mozilla-inbound: 27
* fx-team: 17
* mozilla-central: 6
* b2g-inbound: 3
* try: 2

Platform breakdown:
* linux64: 55

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1217218&startday=2015-10-30&endday=2015-10-30&tree=all

Comment 40

[Intermittent Failures Robot]

45 automation job failures were associated with this bug yesterday.

Repository breakdown:
* mozilla-inbound: 32
* mozilla-central: 7
* try: 3
* fx-team: 2
* b2g-inbound: 1

Platform breakdown:
* linux64: 45

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1217218&startday=2015-10-31&endday=2015-10-31&tree=all

Comment 41

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2255c56d2d00

Comment 42

[Intermittent Failures Robot]

289 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 135
* mozilla-central: 59
* fx-team: 49
* b2g-inbound: 32
* try: 14

Platform breakdown:
* linux64: 289

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1217218&startday=2015-10-26&endday=2015-11-01&tree=all

Comment 43

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2255c56d2d00

Comment 44

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/2255c56d2d00

Comment 45

[Carsten Book [:Tomcat]]

removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion

Comment 46

[Intermittent Failures Robot]

7 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* b2g-inbound: 3
* mozilla-inbound: 2
* fx-team: 2

Platform breakdown:
* linux64: 7

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1217218&startday=2015-11-02&endday=2015-11-08&tree=all