103016 - crash [@nsSocketTransport::OnStatus]

Status: RESOLVED FIXED

(Core :: Networking, defect, P1)

Description

[Darin Fisher]

these comments were added to bug 85578 (which was a different bug)

> ------- Additional Comments From [e-mail] 2001-10-02 14:24 -------
> 
> Adding topcrash keyword and  Trunk [@ nsSocketTransport::OnStatus] to summary
> since Bug 86662 has been marked a dup.  The crash reported there has come > 
> back on the MozillaTrunk as a topcrasher, so reopening.  Here is the latest 
> info:
> 
> nsSocketTransport::OnStatus   31
> 	
	 86662 	 RESO 	 DUPL 	 [e-mail] --- 
> BBID range: 35722446 - 36147325
> Min/Max Seconds since last crash: 604 - 368160
> Min/Max Runtime: 1343 - 368160
> Crash data range: 2001-09-21 to 2001-10-01
> Build ID range: 2001092105 to 2001093011
> 
> Stack Trace: 
> 
> 	 nsSocketTransport::OnStatus
> [d:\builds\seamonkey\mozilla\netwerk\base\src\nsSocketTransport.cpp  line 1846]
> 	 nsSocketReadRequest::OnRead
> [d:\builds\seamonkey\mozilla\netwerk\base\src\nsSocketTransport.cpp  line 2799]
> 	 nsSocketTransport::doReadWrite
> [d:\builds\seamonkey\mozilla\netwerk\base\src\nsSocketTransport.cpp  line 1037]
>  
>  	Source File :
>
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/base/src/nsSocketTransport.cpp
> line : 1846
>      (36115790)	Comments: Reading mail.
>      (36114586)	Comments: Nothing.  But presumably the mail reader was > checking for mail. 
> Also note that during that time my VPN went down  so it's possible it died 
> due to not being able to contact the server?
>      (36110508)	Comments: Crashed over the weekend; mail window was 
> open and periodically
> checking mail  so maybe that?  otherwise  cause unknown.
>      (36074194)	Comments: Nothing.  I left my computer and come back 
> several hours later to
> find it had crashed.  But presumably it was polling for new mail during that 
> time.
>      (36056266)	Comments: It hangs when it's turn on for too long  and > you're doing nothing
> with your pc  (4 hours or so)
>      (35904246)	URL: mainetoday.com
>      (35904246)	Comments: had multiple browser windows open  
> mainetoday.com was the most
> recently used  but was not in the foreground on my machine (I was doing stuff 
> in telnet/ssh windows)
>      (35878003)	Comments: It was just sitting there while I was away 
> from my desk.
>      (35867386)	Comments: left moz runnig for a while and did nothing. 
> Maybe the biff??Access Violation in Necko.dll @ 0xC0000005
> 
> Talkback data shows this is occurring on both Linux and Windows, so changing 
> OS to All.

Comment 1

[Darin Fisher]

Here's another interesting talkback entry:


Source File :
line no:

Build: 2001092514 CrashDate: 2001-09-28 UptimeMinutes: 2292 Total: 2292
OS: Linux 2.4.10
URL:
Comment:
BBID: 36013892

0x00000000 3f59d5f8

Stack Trace:
0x00000000
PrepareAndDispatch
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcstubs.cpp
line 68]
SharedStub
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcstubs.cpp
line 139]
nsSocketTransport::OnStatus
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsSocketTransport.cpp line 1846]
nsSocketReadRequest::OnRead
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsSocketTransport.cpp line 2799]
nsSocketTransport::doReadWrite
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsSocketTransport.cpp line 1037]

Comment 2

[Darin Fisher]

whoops.. ignore that... here's the segment that i meant:

0x00000000 3f59d5f8

Stack Trace:
0x00000000
PrepareAndDispatch
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcstubs.cpp
line 68]
SharedStub
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcstubs.cpp
line 139]
nsSocketTransport::OnStatus
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsSocketTransport.cpp line 1846]
nsSocketReadRequest::OnRead
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsSocketTransport.cpp line 2799]
nsSocketTransport::doReadWrite
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsSocketTransport.cpp line 1037]


Source File :
line no:

Build: 2001092405 CrashDate: 2001-09-28 UptimeMinutes: 1760 Total: 4714
OS: Windows NT 5.0 build 2195
URL:
Comment:
BBID: 36031489

Build: 2001100109 CrashDate: 2001-10-02 UptimeMinutes: 98 Total: 98
OS: Windows NT 5.0 build 2195
URL:
Comment:
BBID: 36162773

Comment 3

[Darin Fisher]

from the talkback reports, it appears that this is a windows only regression.

setting OS = WinNT (though it has been reported under Win98 as well).

Comment 4

[Darin Fisher]

-> 0.9.8

Comment 5

[Darin Fisher]

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

Comment 6

[Darin Fisher]

cc'ing some folks perhaps more familiar with xpcom/proxy or xpconnect stuff.

Comment 7

[David Bradley]

There are two sets of traces that I'm seeing. One that ends in OnStatus and the
other in PrepareAndDisplatch. Both, assuming I've matched up the tb's stacks
with actual source used to build it, appear to point to either a null this for
the nsSocketTransport instance or a deleted instance. I couldn't find a talkback
with parameter information.

Comment 8

[Darin Fisher]

dbradley.. thanks for looking at this.. i suspect that both stack traces are
really the same thing.

so, why might iface_info be null?  what does it mean for GetInterfaceInfo to fail?
do we just need to convert the NS_ASSERTION's into NS_ENSURE_TRUE's?

Comment 9

[John Bandhauer]

It seems more likely to me that the wrapper is dead. The calls using 'self' are
on a dead object. They just happen not to crash. It would make no sense for a
valid wrapper to fail to return a valid interface info in the GetInterfaceInfo
call. Adding an NS_ENSURE_TRUE would just paper over the real problem.

Comment 10

[Darin Fisher]

yeah... ok... it is sounding more and more like we're invoking OnStatus on a
null pointer.  i'll take a closer look at nsSocketTransport to see where/how
mProgressSink could be null.

Comment 11

[Darin Fisher]

oh.. and there it is:

nsSocketTransport::SetNotificationCallbacks(nsIInterfaceRequestor *aCallbacks,...)
{
   mNotificationCallbacks = aCallbacks;
   mProgressSink = 0;
   
   ...
}
this assignment shouldn't happen outside of the socket transport's monitor!

not sure why i didn't see this before :P

patch in hand.

Comment 12

[Darin Fisher]

Attachment: v1.0 patch

Comment 13

[Bradley Baetz (:bbaetz)]

Comment on attachment 58786 [details] [diff] [review]
v1.0 patch

r=bbaetz

Comment 14

[Darin Fisher]

Attachment: v1.1 revised patch

actually, the first patch isn't quite right, since OnProgress and OnStatus are
sometimes called outside the socket transport's monitor.  so, i have to grab a
local reference to mProgressSink while holding the monitor and then release the
monitor before calling out to the progress event sink.

fixing this required some significant changes to the OnStatus/OnProgress
notifications.	what i ended up with is the following:

OnStatus_Locked - for sending out status changes while inside the monitor
(happens as we advance the socket state machine)

OnStatusWithProgress - for sending out status and progress changes while
outside the monitor (happens while reading-from/writing-to the socket).  it
made sense to combine these two notifications into one so i would only have to
grab the progress event sink once.

Comment 15

[John Bandhauer]

Comment on attachment 58793 [details] [diff] [review]
v1.1 revised patch

sr=jband
This is not code I have any experience with. The changes look reasonable. I
trust that darin has done an appropriate amount of testing and that he will get
this reviewed by someone closer to this code who might detect any
inconsistencies with the way this system works.

Comment 16

[Doug Turner (:dougt)]

Comment on attachment 58793 [details] [diff] [review]
v1.1 revised patch

reviewed.

Comment 17

[Darin Fisher]

fixed-on-trunk