David Teller [:Yoric] - still alive but not very active Assignee
	Description • 11 years ago

###!!! 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

	David Teller [:Yoric] - still alive but not very active Assignee
	Comment 1 • 11 years ago

Note: there is a very obvious workaround which consists in buffering the input stream manually.

	(dormant account)
	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

	David Teller [:Yoric] - still alive but not very active 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.

	(dormant account)
	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

	David Teller [:Yoric] - still alive but not very active 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?

	David Teller [:Yoric] - still alive but not very active 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.

	(dormant account)
	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

	(dormant account)
	Comment 8 • 11 years ago

There is no point in implementing readsegments for uncompressed zips only

	Benjamin Smedberg
	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?

	David Teller [:Yoric] - still alive but not very active 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.

	Ed Morley [:emorley]
	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

	Ed Morley [:emorley]
	Comment 14 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/2734d5adbf35

	Carsten Book [:Tomcat]
	Comment 15 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/2734d5adbf35