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)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jaas, Assigned: jaas)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
(deleted),
patch
|
smichaud
:
review+
|
Details | Diff | Splinter Review |
(deleted),
text/plain
|
Details |
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.
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 2•15 years ago
|
||
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+
Comment 3•15 years ago
|
||
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)
Comment 5•15 years ago
|
||
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.
Comment 7•15 years ago
|
||
> 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)
Comment 9•15 years ago
|
||
Looks like this landed yesterday:
http://hg.mozilla.org/mozilla-central/rev/482709fada6c
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.
Description
•