Closed
Bug 446689
Opened 15 years ago
Closed 14 years ago
xpcshell's load() function doesn't close file handle
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: richard.vestal, Assigned: Seno.Aiko)
References
Details
(Keywords: fixed1.9.1, Whiteboard: [fixed1.9.1b4])
Attachments
(1 file, 1 obsolete file)
2.42 KB,
patch
|
mrbkap
:
review+
mrbkap
:
superreview+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.0; SLCC1; .NET CLR 2.0.50727; .NET CLR 3.0.04506; .NET CLR 3.5.21022) Build Identifier: xulrunner cvs source from 20080624 Line 234 opens a file for processing but does not close it after the JS_CompileFileHandleForPrincipals call. Reproducible: Always Steps to Reproduce: 1. Call load function from within javascript 2. Immediately try to delete the file passed load 3. Observe lock issue on file
Reporter | ||
Comment 1•15 years ago
|
||
Line 234 of xpcshell.cpp.
The patch in bug 439110 would fix the load() problem but not two other missing fclose() calls (the -f option and "xpcshell.js").
Status: UNCONFIRMED → NEW
Component: Core → XPConnect
Ever confirmed: true
OS: Windows Vista → All
Product: Rhino → Core
QA Contact: core → xpconnect
Hardware: x86 → All
Summary: xpcshell's Load() function doesn't close file handle → xpcshell's load() function doesn't close file handle
Version: other → Trunk
Attachment #361993 -
Flags: review?(mrbkap)
Updated•14 years ago
|
Attachment #361993 -
Flags: review?(mrbkap)
Comment 4•14 years ago
|
||
Comment on attachment 361993 [details] [diff] [review] add missing fclose calls [Checkin: Comment 8 & 9] >diff -r 688c44602a55 -r dc7117ff7d38 js/src/xpconnect/shell/xpcshell.cpp >@@ -444,16 +444,18 @@ Load(JSContext *cx, JSObject *obj, uintN > file = fopen(filename, "r"); > script = JS_CompileFileHandleForPrincipals(cx, obj, filename, file, > gJSPrincipals); >+ if (file) >+ fclose(file); It seems like it would be more consistent to check for failure and fail early there instead of this null check (like what JS_CompileFile does). Unsetting the review request for now, but I'll stamp the next one, I'm sure!
Assignee: nobody → Seno.Aiko
Attachment #361993 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #362248 -
Flags: review?(mrbkap)
Comment 6•14 years ago
|
||
Comment on attachment 362248 [details] [diff] [review] fail early [See comment 6] >diff -r 688c44602a55 js/src/xpconnect/shell/xpcshell.cpp >+ * FIXME bug 439110: we should report an error when load() fails >+ * but that seems to cause some unit test failures. Eek. I didn't realize this part! Given this, it seems better to keep the current behavior and work on throwing an exception in that bug. Sorry for the extra work.
Attachment #362248 -
Flags: review?(mrbkap)
Comment 7•14 years ago
|
||
Comment on attachment 361993 [details] [diff] [review] add missing fclose calls [Checkin: Comment 8 & 9] This is the way to go for now.
Attachment #361993 -
Attachment is obsolete: false
Attachment #361993 -
Flags: superreview+
Attachment #361993 -
Flags: review+
Keywords: checkin-needed
Comment 8•14 years ago
|
||
Comment on attachment 361993 [details] [diff] [review] add missing fclose calls [Checkin: Comment 8 & 9] http://hg.mozilla.org/mozilla-central/rev/0a341c0bfc3f
Attachment #361993 -
Attachment description: add missing fclose calls → add missing fclose calls
[Checkin: Comment 8]
Updated•14 years ago
|
Blocks: 439110
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Updated•14 years ago
|
Attachment #362248 -
Attachment description: fail early → fail early
[See comment 6]
Attachment #362248 -
Attachment is obsolete: true
Updated•14 years ago
|
Flags: wanted1.9.1?
Comment 9•14 years ago
|
||
Comment on attachment 361993 [details] [diff] [review] add missing fclose calls [Checkin: Comment 8 & 9] http://hg.mozilla.org/releases/mozilla-1.9.1/rev/89df940af254
Attachment #361993 -
Attachment description: add missing fclose calls
[Checkin: Comment 8] → add missing fclose calls
[Checkin: Comment 8 & 9]
Updated•14 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•