395845 - view-source leaks nsCParserNode and nsDeque objects

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Robert Sayre]

Steps to reproduce:

1) ./firefox about:blank
2) view source
3) quit

Leaks 5 nsCParserNode objects, 3 nsDeque objects, and some string buffers.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Ugh. CSharedVSContext's singleton is a static variable and we never clean up its members. That stuff needs to clean up after itself.

Comment 2

[Jesse Ruderman]

This is a large leak.  View-source on Slashdot leaks over 5000 nsStringBuffers.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Possible fix

This just removes the CSharedVSContext in favor of member variables.

Comment 4

[Jesse Ruderman]

This patch fixes all the nsCParserNode and nsDeque leaks but none of the nsStringBuffer leaks.  Is the nsStringBuffer leak a separate bug?

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Actually, given that this is also making it harder to find other leaks I do think we should block on this.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

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).

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 284049 [details] [diff] [review]
Possible fix

Yay for kbizzle!

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Would be good to get this into beta. Should be very safe and fixes a leak we'll hit a lot.

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 284049 [details] [diff] [review]
Possible fix

a=endgame drivers for M9

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Blocking, but wouldn't hold M9 in case that patch causes a regression.

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Checked in. We still need to investigate the nsStringBuffer leaks though.

Comment 13

[Jesse Ruderman]

I filed bug 402633 on the nsStringBuffer leak.