nsCSSLoader::LoadSheet leaking file descriptors.

RESOLVED INVALID

Status

()

Core
CSS Parsing and Computation
RESOLVED INVALID
15 years ago
15 years ago

People

(Reporter: dougt, Assigned: dougt)

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

15 years ago
I am seeing a leak of the file descriptor that is created in
nsCSSLoader::LoadSheet.  I think that when the parser that owns this converter
stream is reinit'ed, we don't properly close the stream.  I am not sure why
releasing the stream directly doesn't solve this problem, but calling |close| on
the stream does force the file descriptors to be freed.
So you're calling close() on the nsIConverterInputStream created on line 1273? 
Or are we talking about the stream NS_NewURI returns?
(Assignee)

Comment 3

15 years ago
Created attachment 124029 [details] [diff] [review]
A fix to the leak

This may just be a bandaide to a deeper problem.
Hmm.. if you close() the stream in LoadSheet(), does that fix the problem too? 
There are two different places the stream in nsCSSScanner could come from, and
it would be good to know which one is the problematic one....

Doug?  Ping?  Any way I can detect the file descriptor leak myself?  If so, I
could probably work on this bug if you don't have time...
(Assignee)

Comment 6

15 years ago
i was just playing around with valgrind browsing different pages.  I noticed
this leak.  
I don't have valgrind.  :(
(Assignee)

Comment 8

15 years ago
I can try to take a look later this week. 
Doug?  ping again?  It'd be good to get some traction on this.....
(Assignee)

Comment 10

15 years ago
bz - I found this when running the xpcom leak detector using a build of minimo
on linux.
That's the XPCOM_MEM_LEAK_LOG stuff?  I could give that a spin... anything in
particular I should load to reproduce?
(Assignee)

Comment 12

15 years ago
right.  that is the log you should look at.  dbaron or waterson has a good
tutorial online regarding how to run it.

Try running chofmann's page loader test.  That is where I saw it.  
What's the URL for chofmann's test?
(Assignee)

Comment 14

15 years ago
www.mozilla.org/quality/browser/buster/

Comment 15

15 years ago
bz: if you are running under linux, then you can see file descriptors in
/proc/<pid>/fd (ls -l is nice because it'll show you the corresponding file
paths for each descriptor).  i usually use this information to spot-check for
leaks, especially when i know what file to look for.
OK, I've failed to reproduce this so far.  Most of the URLs that are supposed to
load multiple pages in the browser buster test don't, as far as I can tell.  I
did go through a bunch of URLs manually, and once even got a nsCSSLoader to
leak, but I don't see any stream leaks in there (and I'm not sure how to even
see an FD in that list, since it's not an XPCOM object).

I did carefully go through all places where the streams that the CSSParser sees
go through, and checked that they are not leaked.  They don't seem to be.  Since
calling Close() on an nsUnicharConverterStream is equivalent to just calling its
destructor, one of those would have to leak in order for Close() to be having
any effect, I would think...

Dougt, did you verify that your patch fixed the leak you were seeing?  Or was it
just a guess?
(Assignee)

Comment 17

15 years ago
my patched fixed the leak.  things change, i guess - marking invalid.  If I see
it again, I will more accurately describe the problem.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → INVALID
ok...  I also checked that none of the CSSLoader code changed how it handles
streams since you filed the bug.  :(  So I may just be totally failing to
trigger something...

You need to log in before you can comment on or make changes to this bug.