Closed Bug 537165 Opened 10 years ago Closed 10 years ago

Don't hold on to file handles in jars

Categories

(Core :: Networking: JAR, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: taras.mozilla, Assigned: taras.mozilla)

References

Details

Attachments

(1 file)

mmap handle makes the os file descriptor redundant. Perhaps eventually we'll be able to get rid of nsJAR altogether.
Attachment #419483 - Flags: review?(alfredkayser)
Duplicate of this bug: 511497
The patch gave me dejavu feeling. I had a similar patch in the pipeline for a potential descriptor leak issue here, but this goes a little bit further (closing the FD directly after mmapping it).

Nice catch anyway.

About nsJar removal, two big things are in nsJAR: the manifest parsing and the zip cache. The rest are wrappers around nsZipArchive. Merging the two files will remove some of this wrapping...
(In reply to comment #2)

> About nsJar removal, two big things are in nsJAR: the manifest parsing and the
> zip cache. The rest are wrappers around nsZipArchive. Merging the two files
> will remove some of this wrapping...

Perhaps removal is too big of a goal. I meant getting rid of most of the useless functionality in it. It will be hard to really get rid of it due to it implementing a public interface.
(In reply to comment #2)
> Nice catch anyway.

Alfred, did you mean to r+ this or is this still waiting to be properly reviewed?
Comment on attachment 419483 [details] [diff] [review]
don't persist the os filehandle

r=ak (apologies for the delay)
Attachment #419483 - Flags: review?(alfredkayser) → review+
Keywords: checkin-needed
Assignee: nobody → tglek
Pushed http://hg.mozilla.org/mozilla-central/rev/8e22cca0841a
Status: NEW → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.