NetUtil.asyncCopy cannot read from a nsIJARInputStream

RESOLVED FIXED in mozilla27

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Yoric, Assigned: Yoric)

Tracking

unspecified
mozilla27
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

###!!! 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)
Flags: needinfo?(taras.mozilla)
(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
Flags: needinfo?(taras.mozilla)
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
Attachment #818084 - Flags: feedback?(benjamin) → feedback+
Pushed this along with a bunch of other checkin-neededs to try:
https://tbpl.mozilla.org/?tree=Try&rev=503271ae46d1
https://hg.mozilla.org/mozilla-central/rev/2734d5adbf35
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.