336892 - Feed preview leaks 1 docshell and 2 documents

Status: VERIFIED FIXED

(Firefox Graveyard :: RSS Discovery and Preview, defect, P2)

Description

[Daniel C]

In a 20060506 Minefield build.

1. Start Minefield with leak-logging enabled.
2. Load the URL in this bug.
3. Quit.
4. Analyse the log.

Leaked document at address 904cf08.
 ... with URI "about:blank".
Leaked document at address 94caed8.
 ... with URI "http://newsrss.bbc.co.uk/rss/newsonline_world_edition/front_page/rss.xml".
 ... with URI "jar:file:///home/djc/mozilla/browser-i686-pc-linux-gnu/firefox-3.0a1.en-US.linux-i686/firefox/chrome/browser.jar!/content/browser/feeds/subscribe.xhtml".
Leaked docshell at address 911e978.
 ... which loaded URI "about:blank".
 ... which loaded URI "http://newsrss.bbc.co.uk/rss/newsonline_world_edition/front_page/rss.xml".
 ... which loaded URI "http://fxfeeds.mozilla.com/rss20.xml".
Summary:
Leaked 0 out of 11 DOM Windows
Leaked 2 out of 42 documents
Leaked 1 out of 5 docshells

Comment 1

[Ria Klaassen (not reading all bugmail)]

In Windows XP I get this:

Leaked docshell at address 264e008.
 ... which loaded URI "about:blank".
 ... which loaded URI "http://www.mozilla.org/projects/minefield/".
 ... which loaded URI "http://fxfeeds.mozilla.com/rss20.xml".
 ... which loaded URI "http://newsrss.bbc.co.uk/rss/newsonline_world_edition/front_page/rss.xml".

Summary:
Leaked 0 out of 8 DOM Windows
Leaked 0 out of 36 documents
Leaked 1 out of 3 docshells

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Interestingly, Leak Monitor 0.1.3 doesn't report any links when opening the above URL or when closing the tab it was opened in.

Comment 3

[Ben Goodger (use ben at mozilla dot org for email)]

marking blocking for investigation

Comment 4

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: patch

Release references for things that we hold onto when we're done with them.

Comment 5

[Darin Fisher]

Comment on attachment 229905 [details] [diff] [review]
patch 

try { ... } finally { this._releaseHandles(); } to avoid this leak if an exception is thrown or if someone adds additional return paths?  Or, do I need to see more diff context to know that there are return paths where these handles should not be released?

r=darin

Comment 6

[Ben Goodger (use ben at mozilla dot org for email)]

fixed-on-trunk

Comment 7

[Ben Goodger (use ben at mozilla dot org for email)]

Daniel, can you check this out with the next nightly build?

If you can verify then I can get this to the branch. 

Comment 8

[Michael Wu [:mwu]]

No leaks when checking http://fxfeeds.mozilla.com/rss20.xml. 

To prove that this patch fixed the leak, I commented out the one call to _releaseHandles and reproduced the leak:

Leaked docshell at address 89ae050.
 ... which loaded URI "about:blank".
 ... which loaded URI "http://newsrss.bbc.co.uk/rss/newsonline_world_edition/front_page/rss.xml".
 ... which loaded URI "http://fxfeeds.mozilla.com/rss20.xml".
Summary:
Leaked 0 out of 7 DOM Windows
Leaked 0 out of 37 documents
Leaked 1 out of 3 docshells

So it appears the leak fix works.

Comment 9

[Robert Sayre]

Attachment: branch patch

Comment 10

[Robert Sayre]

Checking in FeedConverter.js;
/cvsroot/mozilla/browser/components/feeds/src/FeedConverter.js,v  <--  FeedConverter.js
new revision: 1.1.2.12; previous revision: 1.1.2.11
done