1219919 - Add suppressions for Windows-specific content process graphics leaks

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Andrew McCreight [:mccr8]]

See bug 1219916. I'm adding suppressions so Aurora isn't orange.

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: Ignore some non-Nightly texture leaks.

Comment 2

[Eric Rahm [:erahm]]

Comment on attachment 8680873 [details] [diff] [review]
Ignore some non-Nightly texture leaks.

Review of attachment 8680873 [details] [diff] [review]:
-----------------------------------------------------------------

Seems fine, but judging from bug 1219916, comment 0 it sounds like this might not be a stable number?

::: testing/mozbase/mozleak/mozleak/leaklog.py
@@ +75,5 @@
> +        'PTextureChild': 4,
> +        'SharedMemory': 4,
> +        'TextureChild': 4,
> +        'WeakReference<MessageListener>': 8,
> +    })

Is the amount constant during testing or should we add some padding?

Comment 3

[Andrew McCreight [:mccr8]]

(In reply to Eric Rahm [:erahm] from comment #2)
> Seems fine, but judging from bug 1219916, comment 0 it sounds like this
> might not be a stable number?

Yeah, probably not, but I doubt anybody is landing new tests on Aurora so it will at least tide us over for 5 weeks.

> Is the amount constant during testing or should we add some padding?

That's a good question, but oddly enough it does seem to be stable:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-aurora&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified

If it starts getting flaky and exceeding it sometimes I can bump it up more.

Comment 4

[Andrew McCreight [:mccr8]]

This is a test-only change so I think it can also be landed on Aurora without approval.

Comment 5

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e902b5a2882b

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/e902b5a2882b

Comment 7

[Andrew McCreight [:mccr8]]

Attachment: Ignore some non-Nightly texture leaks.

This is just an updated version of the patch for the leaks that show up on Aurora 45, so I'm just going to carry forward erahm's review.

Comment 8

[Andrew McCreight [:mccr8]]

Attachment 8701102 [details] [diff] needs to be landed on Aurora only.

Comment 9

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/01fd84681a25

Comment 10

[Eric Rahm [:erahm]]

Comment on attachment 8701102 [details] [diff] [review]
Ignore some non-Nightly texture leaks.

Review of attachment 8701102 [details] [diff] [review]:
-----------------------------------------------------------------

Looks like this already landed, just clearing the review.

Comment 11

[Andrew McCreight [:mccr8]]

Oops, I meant to just r+ it myself.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

FYI, I'm seeing some similar-looking leaks on an Aurora-as-Beta Try simulation push for Gecko 45.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ffa74919006&group_state=expanded&filter-searchStr=win%20debug%20e10s

Comment 13

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

Relanding the suppression for aurora 46:
https://hg.mozilla.org/releases/mozilla-aurora/rev/b9023e7f7797

Comment 14

[Andrew McCreight [:mccr8]]

These are actually just Windows-specific graphics leaks, but we were only seeing them on Aurora because leak logging was broken there. I'll update this to include more leaks that Bob's patches have revealed.

This also blocks removing the exit(0) for WinXP on Nightly (bug 1242119), because when you do that additional leaks show up. This does not require Bob's patches. I'm guessing that WinXP doesn't have the tighter sandbox we use on Win7 or something.

Comment 15

[Andrew McCreight [:mccr8]]

Attachment: Add suppressions for Windows-specific content process graphics leaks.

Here's a new gigantic suppression list for Windows based on Bob's patch getting leak checking working.

Comment 16

[Andrew McCreight [:mccr8]]

Attachment: Add suppressions for Windows-specific content process graphics leaks.

This is a horribly long list, but it seems better to have the giant list than to not run leak logging on Windows content processes at all. The additional leaks are guarded behind mozinfo.isWin, which means they only apply to Windows. This function is only called for content processes, so it does not affect our leak checking otherwise.

try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bdc3933abda0

There's a failure in e10s dt1, but I've added another SyncObject to the suppression list to account for that.

Comment 17

[Eric Rahm [:erahm]]

Comment on attachment 8715017 [details] [diff] [review]
Add suppressions for Windows-specific content process graphics leaks.

Review of attachment 8715017 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

Comment 18

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3885c33f2013

Comment 19

[Ryan VanderMeulen [:RyanVM]]

(In reply to Pulsebot from comment #18)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/3885c33f2013

I grafted this to Aurora too since it's effectively NPOTB and helps cut down on the Try uplift simulation noise. Also makes it easier to uplift bug 1219369 to Aurora as well.
https://hg.mozilla.org/releases/mozilla-aurora/rev/52d03258c744

Maybe we should consider this and bug 1219369 for Beta45 as well?

Comment 20

[Bob Owen (:bobowen)]

(In reply to Andrew McCreight [:mccr8] from comment #14)

> This also blocks removing the exit(0) for WinXP on Nightly (bug 1242119),
> because when you do that additional leaks show up. This does not require
> Bob's patches. I'm guessing that WinXP doesn't have the tighter sandbox we
> use on Win7 or something.

WinXP doesn't have integrity levels, so the sandbox is pretty weak there at the moment ... only removing Admin rights and a couple of other things.

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3885c33f2013

Comment 22

[Andrew McCreight [:mccr8]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #19)
> I grafted this to Aurora too since it's effectively NPOTB and helps cut down
> on the Try uplift simulation noise.

I don't understand what you mean here. Isn't mozilla-central-as-Aurora going to use the suppression file from m-c?

> Also makes it easier to uplift bug 1219369 to Aurora as well.
> https://hg.mozilla.org/releases/mozilla-aurora/rev/52d03258c744

KWierso already landed an earlier version of these suppressions on Aurora, in comment 13, so this new landing is essentially increasing the leak limit on Aurora. Please back one or the other out (the newer one is better, but it will less work to back out).

> Maybe we should consider this and bug 1219369 for Beta45 as well?

I'm not sure how useful that would be, as it isn't like this has resulted in any leaks being fixed, but I guess it wouldn't hurt. Really, there's a cluster of 5 or so bugs (bug 1091917 and everything blocking it) that are needed to make leak checking really work for Windows content processes.

(In reply to Bob Owen (:bobowen) from comment #20)
> WinXP doesn't have integrity levels, so the sandbox is pretty weak there at
> the moment ... only removing Admin rights and a couple of other things.

I figured that was what was happening. Thanks for confirming.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

(In reply to Andrew McCreight [:mccr8] from comment #22)
> I don't understand what you mean here. Isn't mozilla-central-as-Aurora going
> to use the suppression file from m-c?

I was seeing issues on Aurora-as-Beta too.

> KWierso already landed an earlier version of these suppressions on Aurora,
> in comment 13, so this new landing is essentially increasing the leak limit
> on Aurora. Please back one or the other out (the newer one is better, but it
> will less work to back out).

Done.
https://hg.mozilla.org/releases/mozilla-aurora/rev/a6605dddf40e

> I'm not sure how useful that would be, as it isn't like this has resulted in
> any leaks being fixed, but I guess it wouldn't hurt. Really, there's a
> cluster of 5 or so bugs (bug 1091917 and everything blocking it) that are
> needed to make leak checking really work for Windows content processes.

Doesn't sound worth it then, thanks.

Comment 24

[Andrew McCreight [:mccr8]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #23)
> I was seeing issues on Aurora-as-Beta too.

Does it look like those will be covered by the additional suppressions?

> Done.
> https://hg.mozilla.org/releases/mozilla-aurora/rev/a6605dddf40e

Thanks!

Comment 25

[Ryan VanderMeulen [:RyanVM]]

(In reply to Andrew McCreight [:mccr8] from comment #24)
> Does it look like those will be covered by the additional suppressions?
> 

The Try push I kicked off last night says yes :). I'll let you know if anything changes after today's backout.