Closed Bug 559047 Opened 15 years ago Closed 15 years ago

remove root autorelease pool in Mac OS X appshell

Categories

(Core :: Widget: Cocoa, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jaas, Assigned: jaas)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached patch fix v1.0 (deleted) — Splinter Review
We should consider removing the root autorelease pool in the Mac OS X appshell. Objects that are caught by it effectively leak, just as they would if they were not caught by a root autorelease pool. The root autorelease pool may just be extra work and liability that covers up leak warnings to the console.
Attached file leaked object output (deleted) —
This is a log from a quick run without a root autorelease pool. Using grep and wc you can see that there are 430 objects leaked, 67 at startup. We didn't notice this because the root autorelease pool covered it up. The 67 objects leaked at startup include 6 font objects and a bunch of strings.
Attachment #438757 - Flags: review?(smichaud)
Summary: consider removing root autorelease pool in Mac OS X appshell → remove root autorelease pool in Mac OS X appshell
Comment on attachment 438757 [details] [diff] [review] fix v1.0 This is fine with me, at least as an experiment. But once the root autorelease pool is gone, we'll be obliged to fix all the leaks pretty quickly ... or have a lot of egg on our faces :-) And if (as I suspect) the total size of leaks to the root autorelease pool never gets very large, this might be time better spent elsewhere. Of course if bug 558958 turns out to be caused by leaks to the root autorelease pool, that's a different story -- those leaks are very large. Finally, someone should check that getting rid of the root autorelease pool doesn't have a bad effect on Camino. Note the comment in the nsAppShell destructor above the code that conditionally releases it.
Attachment #438757 - Flags: review?(smichaud) → review+
Blocks: 559073
I tried a similar experiment, running page-cycling tests in several tabs for about 10 minutes. My log showed 66 objects leaked at startup, and a further 328 during shutdown. However, there did not appear to be any ongoing leakage during the course of the run. So I don't think this explains the ongoing leakage shown by bug 558958.
Comment on attachment 438757 [details] [diff] [review] fix v1.0 Requesting sr since landing this will create some alarming output. The alarming output is desired behavior though - it indicates real leaks that we need to fix and which we don't otherwise detect.
Attachment #438757 - Flags: superreview?(benjamin)
Here's a thought: Why don't we postpone landing this patch on the trunk until we've fixed all the obvious, easily reproducible leaks that the root autorelease pool currently masks.
We already have a patch for most of the startup leaks. We can land that at the same time as this, no further need to delay.
> We already have a patch for most of the startup leaks. Where is it?
(In reply to comment #7) > > We already have a patch for most of the startup leaks. > > Where is it? bug 559075
Attachment #438757 - Flags: superreview?(benjamin)
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: