Closed Bug 927366 Opened 7 years ago Closed 7 years ago
Util .async Copy cannot read from a ns IJARInput Stream
###!!! 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
Note: there is a very obvious workaround which consists in buffering the input stream manually.
(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
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.
it's not a common case. Would be good to keep all gzipping off main thread :)...So buffered stream doesnt really help
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)
(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 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+
There is no point in implementing readsegments for uncompressed zips only
Duplicate of this bug: 921574
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?
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: nobody → dteller
Pushed this along with a bunch of other checkin-neededs to try: https://tbpl.mozilla.org/?tree=Try&rev=503271ae46d1
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.