Closed
Bug 927366
Opened 11 years ago
Closed 11 years ago
NetUtil.asyncCopy cannot read from a nsIJARInputStream
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla27
People
(Reporter: Yoric, Assigned: Yoric)
References
Details
Attachments
(1 file, 1 obsolete file)
912 bytes,
patch
|
Yoric
:
review+
|
Details | Diff | Splinter Review |
###!!! ASSERTION: Consumers should be using Read()!: 'Not Reached', file /Users/david/Documents/Code/mc-promise/modules/libjar/nsJARInputStream.cpp, line 232 Relevant part of the stack: thread #1: tid = 0x2003, 0x000000010d60097d libmozalloc.dylib`mozalloc_abort(msg=0x00007fff5fbf6de8) + 93 at mozalloc_abort.cpp:30, stop reason = EXC_BAD_ACCESS (code=1, address=0x0) frame #0: 0x000000010d60097d libmozalloc.dylib`mozalloc_abort(msg=0x00007fff5fbf6de8) + 93 at mozalloc_abort.cpp:30 frame #1: 0x00000001030fdf55 XUL`Abort(aMsg=0x00007fff5fbf6de8) + 21 at nsDebugImpl.cpp:430 frame #2: 0x00000001030fda6b XUL`NS_DebugBreak(aSeverity=1, aStr=0x0000000104dd038b, aExpr=0x0000000104d70fc7, aFile=0x0000000104dd0264, aLine=232) + 1323 at nsDebugImpl.cpp:417 frame #3: 0x00000001006841ae XUL`nsJARInputStream::ReadSegments(this=0x000000010e82c2e0, writer=0x00000001030a5890, closure=0x00007fff5fbf7267, count=1, _retval=0x00007fff5fbf7260)(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*), void*, unsigned int, unsigned int*) + 78 at nsJARInputStream.cpp:232 frame #4: 0x00000001030a5834 XUL`NS_InputStreamIsBuffered(stream=0x000000010e82c2e0) + 52 at nsStreamUtils.cpp:673 frame #5: 0x0000000103092389 XUL`nsIOUtil::InputStreamIsBuffered(this=0x0000000110c861c0, aStream=0x000000010e82c2e0, _retval=0x00007fff5fbf7540) + 121 at nsIOUtil.cpp:17 frame #6: 0x00000001031146f9 XUL`NS_InvokeByIndex(that=0x0000000110c861c0, methodIndex=3, paramCount=2, params=0x00007fff5fbf7528) + 537 at xptcinvoke_x86_64_unix.cpp:162
Assignee | ||
Comment 1•11 years ago
|
||
Note: there is a very obvious workaround which consists in buffering the input stream manually.
Comment 2•11 years ago
|
||
(In reply to David Rajchenbach Teller [:Yoric] from comment #1) > Note: there is a very obvious workaround which consists in buffering the > input stream manually. that's a hack, not a workaround. nsJARInputStream works on a memory buffer already, see how the :Read method is implemented. This would have to be careful with SEH exceptions
Assignee | ||
Comment 3•11 years ago
|
||
If my understanding of that code is correct, Read() uses a memory buffer only in MODE_COPY, i.e. only if the file is STORED rather than DEFLATED. I'm not sure it's a very common case. More generally, the definition of buffered for IOUtils is whether we can call https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIInputStream#readSegments – which is obviously not the case with nsIJARInputStream.
Comment 4•11 years ago
|
||
it's not a common case. Would be good to keep all gzipping off main thread :)...So buffered stream doesnt really help
Assignee | ||
Comment 5•11 years ago
|
||
Trivial patch to avoid the crash. Returning NS_ERROR_NOT_IMPLEMENTED, as per .idl specifications, should be sufficient. Now, I guess I could implement the method if the entry is simply STORED, rather than DEFLATED. Is there any interest in this?
Attachment #818084 -
Flags: feedback?(benjamin)
Flags: needinfo?(taras.mozilla)
Assignee | ||
Comment 6•11 years ago
|
||
(In reply to Taras Glek (:taras) from comment #4) > it's not a common case. Would be good to keep all gzipping off main thread > :)...So buffered stream doesnt really help No, buffered stream most likely doesn't do anything useful, except avoiding the crash.
Comment 7•11 years ago
|
||
Comment on attachment 818084 [details] [diff] [review] Don't crash if we attempt to check whether nsIJARInputStream is buffered This seems fine
Attachment #818084 -
Flags: review+
Comment 8•11 years ago
|
||
There is no point in implementing readsegments for uncompressed zips only
Flags: needinfo?(taras.mozilla)
Comment 10•11 years ago
|
||
Comment on attachment 818084 [details] [diff] [review] Don't crash if we attempt to check whether nsIJARInputStream is buffered I read the bug, but it's not clear to me what the situation is where we are actually calling ReadSegments but shouldn't crash. Have you got a stack which explains this?
Assignee | ||
Comment 11•11 years ago
|
||
NetUtil.asyncCopy calls immediately nsIOUtil::InputStreamIsBuffered to determine whether the input stream is buffered. The rest of the stack is in comment 0: InputStreamIsBuffered determines whether an input stream is buffered by calling ReadSegments, which in the case of nsJARInputStream calls MOZ_CRASH.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → dteller
Updated•11 years ago
|
Attachment #818084 -
Flags: feedback?(benjamin) → feedback+
Assignee | ||
Comment 12•11 years ago
|
||
Attachment #818084 -
Attachment is obsolete: true
Attachment #819196 -
Flags: review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 13•11 years ago
|
||
Pushed this along with a bunch of other checkin-neededs to try: https://tbpl.mozilla.org/?tree=Try&rev=503271ae46d1
Comment 14•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2734d5adbf35
Keywords: checkin-needed
Comment 15•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2734d5adbf35
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in
before you can comment on or make changes to this bug.
Description
•