89595 - Browser crashes each time while showing this page - Trunk [@ MSVCRT.DLL - nsInputStreamTee::WriteSegmentFun]

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect, P2)

Description

[Yakov Markovitch]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.2) Gecko/20010628
BuildID: 2001062815

Browser crashes each time while showing the chart picture on the page
http://gcc.gnu.org/benchresults/aov.html

Reproducible: Always
Steps to Reproduce:
1. Open the URL

Actual Results:  Browser crashes with standard OS "illegal memory access" dialog
box.

Comment 1

[Markus Gerstel]

confirmed on w2k 2001070404

TB32593748E

Comment 2

[Markus Gerstel]

It's the image.

TB32593953W

 -> ImageLib

Netscape 4.77 shows half of the image, Internet Explorer shows nothing

Comment 3

[Chase Tingley]

WFM Linux 2001070206 and 20010705 (cvs).  Well, WFM as in "displays half an
image."  

Comment 4

[Francisco León]

Confirmed, linux, 0.9.2
Talkback: TB32596438Q

Comment 5

[Asa Dotzler [:asa]]

Incident ID 32596438
Stack Signature libc.so.6 + 0x2ce54 (0x404eae54) 0d130da9
Bug ID
Trigger Time 2001-07-06 09:41:19
User Comments was seeing a webpage of gnu (listed at a recent bug)
Build ID 2001062823
Product ID Netscape6.10
Platform ID LinuxIntel
Stack Trace
libc.so.6 + 0x2ce54 (0x404eae54) 

Comment 6

[Francisco León]

Looks too similar like bug 88744 (the stack trace)

Comment 7

[aroy]

WFM on Solaris Build 2001070522. --Roy

Comment 8

[Asa Dotzler [:asa]]

reassign.

Comment 9

[Jacek Piskozub]

Crash on 0.9.2 branch 20010706 build with WindowsME.

Talback incident ID TB32606402Z

Comment 10

[Markus Gerstel]

*** Bug 89746 has been marked as a duplicate of this bug. ***

Comment 11

[Stuart Parmenter]

over to dougt.  nsPipe::nsPipeInputStream::ReadSegments doesn't return the error 
that is returned to by the writer function thing.

Comment 12

[Doug Turner (:dougt)]

Attachment: returns writer error.

Comment 13

[Doug Turner (:dougt)]

I am not sure that this is the correct fix, but it does address the problem.  I 
am not sure why both ReadSegment and WriteSegment ignore the user-defined 
callback return value.  

rpotts could you review, please?  Should we be also be doing something similar 
in WriteSegments (although it uses mCondition, but I don't think that is good 
enough).

Comment 14

[rpotts (gone)]

hey doug,

i think that you are hitting one of the old semantics of the pipe code :-)  

In the very beginning, warren made the assumption that if any data could be read 
from the pipe that the return code should be NS_OK...  This means that the 
"writer" functions must be prepared to return the SAME failure code on 
subsequent calls... so that once all of the data has been drained from the pipe 
that the real error code can be propagated via:

    return *readCount == 0 ? rv : NS_OK;

If "writer" functions obey this convention hten everything is fine :-) 
otherwise, well bad things happen.

I'm not sure what the rammifications will be if an NS_ERROR code is returned 
along with data..  I wouldn't be surprised if there is code that makes the 
assumption that if data is *only* returned with NS_OK...

Could Pav change his "writer" function to do the right thing?

Comment 15

[Keyser Sose]

*** Bug 89393 has been marked as a duplicate of this bug. ***

Comment 16

[Doug Turner (:dougt)]

Your right.  If he hit and error, he can cancel the request via the closure.  
reassigning.

Comment 17

[Jay Patel [:jay]]

I found A LOT of crashes submitted to Talkback under Windows 2k... Here is the
stack trace of one of the entries:

Incident ID 32593748 
MSVCRT.DLL + 0x1f64b (0x7801f64b) 
nsInputStreamTee::WriteSegmentFun
[d:\builds\seamonkey\mozilla\xpcom\io\nsInputStreamTee.cpp, line 82] 
nsPipe::nsPipeInputStream::ReadSegments
[d:\builds\seamonkey\mozilla\xpcom\io\nsPipe2.cpp, line 412] 
nsInputStreamTee::ReadSegments
[d:\builds\seamonkey\mozilla\xpcom\io\nsInputStreamTee.cpp, line 138] 
nsPNGDecoder::WriteFrom
[d:\builds\seamonkey\mozilla\modules\libpr0n\decoders\png\nsPNGDecoder.cpp, line
164] 
ReadDataOut
[d:\builds\seamonkey\mozilla\modules\libpr0n\decoders\png\nsPNGDecoder.cpp,
line 139] 
nsCOMPtr_base::assign_with_AddRef
[d:\builds\seamonkey\mozilla\xpcom\base\nsCOMPtr.cpp, line 57] 
MSVCRT.DLL + 0x1426 (0x78001426) 
ProxyListener::OnDataAvailable
[d:\builds\seamonkey\mozilla\modules\libpr0n\src\imgLoader.cpp, line 387] 
nsStreamListenerTee::OnDataAvailable
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsStreamListenerTee.cpp, line 57] 
nsHttpChannel::OnDataAvailable
[d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHttpChannel.cpp, line
2146] 
nsOnDataAvailableEvent::HandleEvent
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsStreamListenerProxy.cpp, line 185] 
PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 591] 
PL_ProcessPendingEvents [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c,
line 524] 
_md_EventReceiverProc [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line
1072] 
nsAppShellService::Run
[d:\builds\seamonkey\mozilla\xpfe\appshell\src\nsAppShellService.cpp,
line 419] 
main1 [d:\builds\seamonkey\mozilla\xpfe\bootstrap\nsAppRunner.cpp, line 1181] 
main [d:\builds\seamonkey\mozilla\xpfe\bootstrap\nsAppRunner.cpp, line 1481] 
WinMain [d:\builds\seamonkey\mozilla\xpfe\bootstrap\nsAppRunner.cpp, line 1499] 
WinMainCRTStartup() 
KERNEL32.DLL + 0x17d08 (0x77e87d08) 

Most of the entries with this stack trace metioned this bug and also submitted
http://gcc.gnu.org/benchmarks/ as the url.  Actually, I'm just going to paste in
a couple of the entries from the Talkback topcrash report:

MSVCRT.DLL + 0x1f64b (0x7801f64b) 8164cf2c
         line 
        Build: 2001070710 CrashDate: 2001-07-08 UptimeMinutes: 241  Total: 241 
        OS: Windows NT  5.0 build 2195
         Detailed : http://climate/reports/incidenttemplate.cfm?bbid=32649932
         StackTrace:
http://climate/reports/stackcommentemail.cfm?dynamicBBID=32649932
     (32649932) URL: http://gcc.gnu.org/benchresults/aov.html

MSVCRT.DLL + 0x1f64b (0x7801f64b) 310ac88c
         line 
        Build: 2001070609 CrashDate: 2001-07-07 UptimeMinutes: 172  Total: 172 
        OS: Windows NT  5.0 build 2195
         Detailed : http://climate/reports/incidenttemplate.cfm?bbid=32636615
         StackTrace:
http://climate/reports/stackcommentemail.cfm?dynamicBBID=32636615
     (32636615) URL: http://gcc.gnu.org/benchmarks/
     (32636615) Comments: Browsing with 2 windows: gcc.gnu.org and grosbill.fr

MSVCRT.DLL + 0x1f64b (0x7801f64b) 785a2037
         line 
        Build: 2001070509 CrashDate: 2001-07-06 UptimeMinutes: 282  Total: 532 
        OS: Windows NT  5.0 build 2195
         Detailed : http://climate/reports/incidenttemplate.cfm?bbid=32593748
         StackTrace:
http://climate/reports/stackcommentemail.cfm?dynamicBBID=32593748
     (32593748) Comments: Bug #89595

Adding topcrash keyword and Trunk [@ MSVCRT.DLL -
nsInputStreamTee::WriteSegmentFun] to summary.  This is coming up in large
numbers in the Talkback data, but they could all be from the same person testing
this particular crash...so prioritize accordingly.

Comment 18

[Jay Patel [:jay]]

This doesn't look like a topcrasher, but Talkback data shows a couple of crashes
with recent builds.  Here is one of them:

Incident ID 34588446
Stack Signature MSVCRT.DLL + 0x1e7b4 (0x7801e7b4) 5dcf7e4b
Bug ID
Trigger Time 2001-08-27 12:16:27
Email Address
User Comments
Build ID 2001082706
Product ID MozillaTrunk
Platform ID Win32
Trigger Reason Access violation
Stack Trace
MSVCRT.DLL + 0x1e7b4 (0x7801e7b4)
nsInputStreamTee::WriteSegmentFun
[d:\builds\seamonkey\mozilla\xpcom\io\nsInputStreamTee.cpp, line 82]
nsPipe::nsPipeInputStream::ReadSegments
[d:\builds\seamonkey\mozilla\xpcom\io\nsPipe2.cpp, line 412]
nsInputStreamTee::ReadSegments
[d:\builds\seamonkey\mozilla\xpcom\io\nsInputStreamTee.cpp, line 138]
nsPNGDecoder::WriteFrom
[d:\builds\seamonkey\mozilla\modules\libpr0n\decoders\png\nsPNGDecoder.cpp, line
164]
ReadDataOut
[d:\builds\seamonkey\mozilla\modules\libpr0n\decoders\png\nsPNGDecoder.cpp, line
139]
KERNEL32.DLL + 0xb9b6 (0xbff7b9b6)
ProxyListener::OnDataAvailable
[d:\builds\seamonkey\mozilla\modules\libpr0n\src\imgLoader.cpp, line 402]
nsStreamListenerTee::OnDataAvailable
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsStreamListenerTee.cpp, line 57]
nsHttpChannel::OnDataAvailable
[d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHttpChannel.cpp, line 2232]
nsOnDataAvailableEvent::HandleEvent
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsStreamListenerProxy.cpp, line 188]
PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 591]
PL_ProcessPendingEvents [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c,
line 524]
_md_EventReceiverProc [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line
1072]
KERNEL32.DLL + 0x242e7 (0xbff942e7)
0x00688b5a 

Comment 19

[bclinger]

Clicking on a link, Build 2001092103 and several before it have a fatal
"msvcrt.dll" error at the go.to/speak-out site and several others around the
world also.

Comment 20

[Terri Preston]

Crashes w2k build 2001-09-21-05-0.9.4 (Incident #35695588) Image comes up fine 
linux build 2001-09-21-04-0.9.4, Image is distorted but the browser does not 
crash Mac OS X build 2001-09-21-04-0.9.4

Comment 21

[Mitch]

Crashes 0.9.4+ build 2001100622.

DOESN'T crash 0.9.5+ build 2001101022, but libpng gives errors in console
window. Looks like a bad png ?

$ !LD
LD_LIBRARY_PATH=/opt/gnome-1.4/lib ./mozilla
./run-mozilla.sh ./mozilla-bin
MOZILLA_FIVE_HOME=.
  LD_LIBRARY_PATH=.:./plugins:/opt/gnome-1.4/lib
     LIBRARY_PATH=.:./components
       SHLIB_PATH=.
          LIBPATH=.
       ADDON_PATH=.
      MOZ_PROGRAM=./mozilla-bin
      MOZ_TOOLKIT=
        moz_debug=0
     moz_debugger=
libpng error: Decompression Error
libpng error: Decompression Error

Crashes 0.9.4+ build 

Comment 22

[Terri Preston]

This bug is fixed on linux build 2001110112, w2k build 2001110103 and mac OS X
build 2001103113 (there is no crash)  The image is not displayed properly on Mac
OS X, but that is bug 103934

Comment 23

[Terri Preston]

I take that back, crashes win XP build 2001110209

Comment 24

[Matthias Versen [:Matti]]

*** Bug 111996 has been marked as a duplicate of this bug. ***

Comment 25

[Stuart Parmenter]

Attachment: patch to hopefully fix the crash

darin thinks that by moving the setjmp into a non-stdcall method it should fix
the problem.  i can't reproduce the crash though.

Comment 26

[Darin Fisher]

pavlov:

i think i was leading you in the wrong direction w/ the setjmp/stdcall foo...
all the documentation i could find on setjmp seems to suggest that it shouldn't
depend on function calling convention.  so, i thought about this some more, and
what rpotts said makes sense to me.

the method ReadDataOut can be called several times, during a call to
ReadSegments.  if ReadDataOut succeeds on the first call, but fails on a
subsequent call, then ReadSegments will return NS_OK.  this screws up your error
handling, and it looks like you were expecting ReadSegments to return the error
returned from ReadDataOut.  but, that won't always be the case.

as rpotts said, this is the convention for pipes, and it is that way for a very
good reason.  lots would break if we changed the convention and functionality
would be lost.

this means that in ReadDataOut you have to record the error code someplace, so
that if ReadDataOut is ever called again, you'll be able to return the error
code again... and not call ProcessData.  you could also check this error code
after ReadSegments returns, and then if NS_FAILED(the_error_code) return
the_error_code.  then it looks like the imgRequest code would do the right thing.

Comment 27

[Nivedita (gone)]

*** Bug 118233 has been marked as a duplicate of this bug. ***

Comment 28

[Stuart Parmenter]

Attachment: Patch to always return NS_OK to the pipe code

Comment 29

[Stuart Parmenter]

Attachment: Updated patch

Darin says it is ok to return an error from ReadDataOut, so we'll do that and
still use mError to determine the result in WriteFrom

Comment 30

[Stuart Parmenter]

adding topcrash from one of the dups

Comment 31

[scottputterman]

Moving Netscape owned 0.9.9 and 1.0 bugs that don't have an nsbeta1, nsbeta1+,
topembed, topembed+, Mozilla0.9.9+ or Mozilla1.0+ keyword.  Please send any
questions or feedback about this to [e-mail].  You can search for
"Moving bugs not scheduled for a project" to quickly delete this bugmail.

Comment 32

[Stuart Parmenter]

this is topembed with a patch.  why are we moving this to 1.2?

Comment 33

[Stuart Parmenter]

er, topcrash, not topembed

Comment 34

[Gagan]

per adt, critical for nsbeta1. hence plus.

Comment 35

[Darin Fisher]

Comment on attachment 72177 [details] [diff] [review]
Updated patch

sr=darin

Comment 36

[Doug Turner (:dougt)]

Comment on attachment 72177 [details] [diff] [review]
Updated patch

r=dougt

Comment 37

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 72177 [details] [diff] [review]
Updated patch

a=roc+moz

Comment 38

[Stuart Parmenter]

checked in

Comment 39

[Diego Biurrun]

It's nice to know that we do not crash at the image anymore, but why don't we
render it?  Netscape 4.77 and Opera do render it...

Comment 40

[Terri Preston]

Verified no crash on win 2k build 2002032603, Mac OS X build 2002032608 and
linux build 2002032608

Comment 41

[Jaime Rodriguez, Jr.]

jay - this crash is occurring on the branch, right?

Comment 42

[Jay Patel [:jay]]

Jaime:  I have not seen any crashes like this in Talkback data for the Gecko1.0
Branch.  It looks like it might have been fixed on the Trunk before we branched
for Gecko1.0.

Comment 43

[scottputterman]

Based on Jay's comments and the fact that the patch was checked in in March
prior to the branch, I'm removing the adt1.0.1 and adt markings.  Please put
them back if  this still needs to land on the branch.