Closed Bug 395845 Opened 17 years ago Closed 17 years ago

view-source leaks nsCParserNode and nsDeque objects

Categories

(Core :: DOM: HTML Parser, defect)

Product:
Core
Component:
DOM: HTML Parser
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: sayrer, Assigned: mrbkap)

References

Details

(Keywords: memory-leak)

Attachments

(1 file)

Possible fix
(deleted), patch
sicking
: review+
sicking
: superreview+
beltzner
: approvalM9+
beltzner
: approval1.9+
Details | Diff | Splinter Review
Steps to reproduce: 1) ./firefox about:blank 2) view source 3) quit Leaks 5 nsCParserNode objects, 3 nsDeque objects, and some string buffers.
Ugh. CSharedVSContext's singleton is a static variable and we never clean up its members. That stuff needs to clean up after itself.
This is a large leak. View-source on Slashdot leaks over 5000 nsStringBuffers.
Flags: blocking1.9?
Attached patch Possible fix (deleted) — — Splinter Review
This just removes the CSharedVSContext in favor of member variables.
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
This patch fixes all the nsCParserNode and nsDeque leaks but none of the nsStringBuffer leaks. Is the nsStringBuffer leak a separate bug?
Not a blocker given that this seems to only happen for view-source, which most users don't use. Still would be really good to have this fixed. Blake, do you know the answer to comment 4? If it is a separate bug we need to get that filed.
Flags: blocking1.9? → blocking1.9-
Whiteboard: [wanted-1.9]
Actually, given that this is also making it harder to find other leaks I do think we should block on this.
Flags: blocking1.9- → blocking1.9?
Whiteboard: [wanted-1.9]
Blocks: 402335
Comment on attachment 284049 [details] [diff] [review] Possible fix This is patch is good as far as it goes, but we'll need a new bug to track the string buffer leaks that Jesse is still seeing (I suspect it has to do with the CIndirectTextToken stuff, but haven't found time to try to track it down).
Attachment #284049 - Flags: superreview?(jonas)
Attachment #284049 - Flags: review?(jonas)
Comment on attachment 284049 [details] [diff] [review] Possible fix Yay for kbizzle!
Attachment #284049 - Flags: superreview?(jonas)
Attachment #284049 - Flags: superreview+
Attachment #284049 - Flags: review?(jonas)
Attachment #284049 - Flags: review+
Would be good to get this into beta. Should be very safe and fixes a leak we'll hit a lot.
Target Milestone: --- → mozilla1.9 M9
Attachment #284049 - Flags: approvalM9?
Comment on attachment 284049 [details] [diff] [review] Possible fix a=endgame drivers for M9
Attachment #284049 - Flags: approvalM9?
Attachment #284049 - Flags: approvalM9+
Attachment #284049 - Flags: approval1.9+
Blocking, but wouldn't hold M9 in case that patch causes a regression.
Flags: blocking1.9? → blocking1.9+
Target Milestone: mozilla1.9 M9 → ---
Checked in. We still need to investigate the nsStringBuffer leaks though.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
I filed bug 402633 on the nsStringBuffer leak.
Summary: view-source leaks → view-source leaks nsCParserNode and nsDeque objects
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: