113949 - Trunk M096 crash [@ nsFileTransport::Process] [@ nsProxyObject::Post]

Status: RESOLVED FIXED

(Core :: Networking, defect, P1)

Description

[Jay Patel [:jay]]

There are a few of these crashes in recent MozillaTrunk builds.  The stack trace
is only one line, so I don't know how helpful it will be in debugging...but
here's the latest from Talkback:

[ 8   nsFileTransport::Process 5b97409e - nsFileTransport::Process ]
 
     Crash date range: 2001-12-03 to 2001-12-05
     Min/Max Seconds since last crash: 539 - 337595
     Min/Max Runtime: 4435 - 337595
     Keyword List :  
     Count   Platform List 
     6   Windows NT 5.0 build 2195
     2   Windows 98 4.90 build 73010104
 
     Count   Build Id List 
     2   2001120309
     1   2001120506
     1   2001120306
     1   2001120211
     1   2001113009
     1   2001113006
     1   2001112914
 
     No of Unique Users         8
 
 Stack trace(Frame) 

	 nsFileTransport::Process
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsFileTransport.cpp  line 853]  
 
     (66040)	URL: back 2 google.com from www.aceweekly.com
     (66040)	Comments: back from  www.aceweekly.com to google..first crash in a long to
tho!!!
     (27179)	URL: www.amiga.org & www.x13design.com/bipolar/
     (27179)	Comments: www.x13design.com/bipolar/ had stalled during loading  i attempted
to open a new navigator window using the QuickLaunch menu (in the taskbar) and
the browser crashed as www.amiga.org was trying to load.
     (9781)	Comments: mapquest

I'll update this with more info as I get it.

Comment 1

[neeti]

Doug, Darin,
 Should bbaetz be getting this bug? Who is looking at FileTransport stuff now?
Neeti

Comment 2

[Darin Fisher]

-> me (i'll look into this... i recently made some changes that /might/ have
resulted in this)

Comment 3

[Darin Fisher]

one more try...

Comment 4

[Darin Fisher]

-> 0.9.7 (maybe)

Comment 5

[Darin Fisher]

not much of a stack trace... if only there was a line number ;)

Comment 6

[Darin Fisher]

-> moz 1.0 (until we have more info)

Comment 7

[Darin Fisher]

this talkback link

http://climate/reports/SingleIncidentInfo.cfm?dynamicBBID=159724

gives this stack trace:

nsFileTransport::Process
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsFileTransport.cpp, line 853]
nsFileTransport::Run
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsFileTransport.cpp, line 637]
nsThreadPoolRunnable::Run
[d:\builds\seamonkey\mozilla\xpcom\threads\nsThread.cpp, line 908]
nsThread::Main [d:\builds\seamonkey\mozilla\xpcom\threads\nsThread.cpp, line 136]
_PR_NativeRunThread [../../../../../pr/src/threads/combined/pruthr.c, line 435]
msvcrt.dll + 0x27fb8 (0x77c37fb8)
kernel32.dll + 0x202ed (0x77e802ed)

Comment 8

[Darin Fisher]

i know exactly what the problem is... i fixed a similar bug in the socket
transport.  the problem is that the file transport's notification callbacks can
be cleared from the UI thread while the file transport is using them on a file
transport thread.  we just need to protect nsFileTransport::mProgressSink with
the file transport's lock.

-> 0.9.7 (patch in hand)

Comment 9

[Darin Fisher]

Attachment: v1.0 patch

this patch should fix this crash.

Comment 10

[Darin Fisher]

dougt: can you r= this patch?  thx!

Comment 11

[Doug Turner (:dougt)]

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

fine.

Comment 12

[Darin Fisher]

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

Comment 13

[Scott MacGregor]

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

sr=mscott pending the answer to my question. I couldn't quite tell from the
context of the diff why we had to add a lock on mLock in
SetNotificationsCallbacks in addtion to passing in the progress event sink each
time into ::Process

Comment 14

[Darin Fisher]

mscott: SetNotificationCallbacks can be called on the main thread, while Process
is running, and assignment into a COMPtr is not atomic... there's the assignment
of the pointer value (which is atomic) and then there's also the AddRef call. 
together these aren't atomic, so we need to protect all access to mProgressSink.

Comment 15

[Darin Fisher]

fixed-on-trunk

Comment 16

[Darin Fisher]

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

Comment 17

[Jay Patel [:jay]]

Adding M096 and [@ nsProxyObject::Post] to summary for tracking, since bug
107843 was marked a dup.

Comment 18

[Doug Turner (:dougt)]

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

Comment 19

[Jay Patel [:jay]]

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

Comment 20

[Doug Turner (:dougt)]

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