Closed
Bug 206781
Opened 21 years ago
Closed 21 years ago
nsCSSLoader::LoadSheet leaking file descriptors.
Categories
(Core :: CSS Parsing and Computation, defect)
Tracking
()
RESOLVED
INVALID
People
(Reporter: dougt, Assigned: dougt)
Details
Attachments
(1 file)
529 bytes,
patch
|
Details | Diff | Splinter Review |
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.
Comment 1•21 years ago
|
||
So you're calling close() on the nsIConverterInputStream created on line 1273? Or are we talking about the stream NS_NewURI returns?
Comment 2•21 years ago
|
||
I meant NS_OpenURI.
Assignee | ||
Comment 3•21 years ago
|
||
This may just be a bandaide to a deeper problem.
Comment 4•21 years ago
|
||
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....
Comment 5•21 years ago
|
||
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•21 years ago
|
||
i was just playing around with valgrind browsing different pages. I noticed this leak.
Comment 7•21 years ago
|
||
I don't have valgrind. :(
Assignee | ||
Comment 8•21 years ago
|
||
I can try to take a look later this week.
Comment 9•21 years ago
|
||
Doug? ping again? It'd be good to get some traction on this.....
Assignee | ||
Comment 10•21 years ago
|
||
bz - I found this when running the xpcom leak detector using a build of minimo on linux.
Comment 11•21 years ago
|
||
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•21 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.
Comment 13•21 years ago
|
||
What's the URL for chofmann's test?
Assignee | ||
Comment 14•21 years ago
|
||
www.mozilla.org/quality/browser/buster/
Comment 15•21 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.
Comment 16•21 years ago
|
||
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•21 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
Closed: 21 years ago
Resolution: --- → INVALID
Comment 18•21 years ago
|
||
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.
Description
•