If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Add suppressions for Windows-specific content process graphics leaks

RESOLVED FIXED in Firefox 44, Firefox OS v2.5

Status

()

Core
Graphics: Layers
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

Trunk
mozilla47
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 fixed, firefox45 fixed, firefox46 fixed, firefox47 fixed, b2g-v2.5 fixed, b2g-master affected)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

2 years ago
See bug 1219916. I'm adding suppressions so Aurora isn't orange.
(Assignee)

Comment 1

2 years ago
Created attachment 8680873 [details] [diff] [review]
Ignore some non-Nightly texture leaks.
Attachment #8680873 - Flags: review?(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?
Attachment #8680873 - Flags: review?(erahm) → review+
(Assignee)

Comment 3

2 years ago
(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.
(Assignee)

Comment 4

2 years ago
This is a test-only change so I think it can also be landed on Aurora without approval.
Keywords: checkin-needed
https://hg.mozilla.org/releases/mozilla-aurora/rev/e902b5a2882b
Keywords: checkin-needed
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/e902b5a2882b
status-b2g-v2.5: --- → fixed
status-b2g-master: --- → affected
status-firefox44: --- → fixed

Updated

2 years ago
See Also: → bug 1232549
(Assignee)

Comment 7

2 years ago
Created attachment 8701102 [details] [diff] [review]
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.
Attachment #8701102 - Flags: review?(erahm)
(Assignee)

Comment 8

2 years ago
Attachment 8701102 [details] [diff] needs to be landed on Aurora only.
Keywords: checkin-needed
https://hg.mozilla.org/releases/mozilla-aurora/rev/01fd84681a25
status-firefox45: affected → fixed
Keywords: checkin-needed
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.
Attachment #8701102 - Flags: review?(erahm) → review+
(Assignee)

Comment 11

2 years ago
Oops, I meant to just r+ it myself.
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
Relanding the suppression for aurora 46:
https://hg.mozilla.org/releases/mozilla-aurora/rev/b9023e7f7797
status-firefox46: --- → fixed
(Assignee)

Comment 14

2 years ago
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.
Blocks: 1219369, 1242119
Summary: Add suppressions for non-Nightly texture leaks. → Add suppressions for Windows-specific graphics leaks
(Assignee)

Updated

2 years ago
Summary: Add suppressions for Windows-specific graphics leaks → Add suppressions for Windows-specific content process graphics leaks
(Assignee)

Comment 15

2 years ago
Created attachment 8714983 [details] [diff] [review]
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.
(Assignee)

Comment 16

2 years ago
Created attachment 8715017 [details] [diff] [review]
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.
Attachment #8715017 - Flags: review?(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
Attachment #8715017 - Flags: review?(erahm) → review+

Comment 18

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3885c33f2013
(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?
Flags: needinfo?(continuation)

Comment 20

2 years ago
(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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3885c33f2013
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
(Assignee)

Comment 22

2 years ago
(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.
Flags: needinfo?(continuation) → needinfo?(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.
Flags: needinfo?(ryanvm)
(Assignee)

Comment 24

2 years ago
(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!
(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.
(Assignee)

Updated

2 years ago
Attachment #8714983 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1232549
(Assignee)

Updated

2 years ago
Blocks: 1247025
You need to log in before you can comment on or make changes to this bug.