Feed preview leaks 1 docshell and 2 documents

VERIFIED FIXED in Firefox 2 beta2

Status

()

P2
normal
VERIFIED FIXED
13 years ago
8 years ago

People

(Reporter: djcater+bugzilla, Assigned: bugs)

Tracking

({fixed1.8.1, memory-leak})

Trunk
Firefox 2 beta2
x86
Linux
fixed1.8.1, memory-leak
Points:
---
Bug Flags:
blocking-firefox2 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mustfix], URL)

Attachments

(2 attachments)

(Reporter)

Description

13 years ago
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
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
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.
Assignee: nobody → bugs
Priority: -- → P2
Whiteboard: [swag:3d]
Target Milestone: --- → Firefox 2 beta1
marking blocking for investigation
Flags: blocking-firefox2+

Updated

12 years ago
Target Milestone: Firefox 2 beta1 → Firefox 2 beta2

Updated

12 years ago
Whiteboard: [swag:3d] → [swag:3d][mustfix]
Created attachment 229905 [details] [diff] [review]
patch 

Release references for things that we hold onto when we're done with them.
Attachment #229905 - Flags: review?(darin)

Comment 5

12 years ago
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
Attachment #229905 - Flags: review?(darin) → review+
fixed-on-trunk
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Daniel, can you check this out with the next nightly build?

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

Updated

12 years ago
Whiteboard: [swag:3d][mustfix] → [mustfix][needs verification]

Comment 8

12 years ago
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.
Status: RESOLVED → VERIFIED

Updated

12 years ago
Attachment #229905 - Flags: approval1.8.1?

Updated

12 years ago
Attachment #229905 - Flags: approval1.8.1?

Comment 9

12 years ago
Created attachment 230853 [details] [diff] [review]
branch patch

Updated

12 years ago
Attachment #230853 - Flags: approval1.8.1?

Updated

12 years ago
Attachment #230853 - Flags: approval1.8.1? → approval1.8.1+

Comment 10

12 years ago
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
Keywords: fixed1.8.1
(Reporter)

Updated

8 years ago
Whiteboard: [mustfix][needs verification] → [mustfix]
You need to log in before you can comment on or make changes to this bug.