54871 - XPInstall creates native eventqueue on other than the main thread

Status: VERIFIED FIXED

(Core :: XPCOM, defect, P3)

Description

[(not reading, please use [e-mail] instead)]

to reproduce:

use a mozilla (not commercial) linux branch build.  (I used my
Netscape_20000922_BRANCH branch build, but using the 2000100109 MN6 build from
ftp.mozilla.org also crashed.)

start up and open
http://docs.iplanet.com/docs/manuals/psm/psm-mozilla/index.html

click on "Install Netscape PSM for Linux"

it seems to crash after the "XPInstall Results" step.

#0  nsCOMPtr<nsISupportsArray>::operator-> (this=0x2c)
    at ../../../../dist/include/nsCOMPtr.h:649
#1  0x409f223c in nsHTTPPipelinedRequest::GetCurrentRequest (this=0x0, 
    o_Req=0xbffff5d4) at nsHTTPRequest.cpp:1095
#2  0x409f4649 in nsHTTPServerListener::OnStartRequest (this=0x8af02a8, 
    channel=0x8abb1e4, i_pContext=0x8b8ea58) at nsHTTPResponseListener.cpp:650
#3  0x409889e8 in nsOnStartRequestEvent::HandleEvent (this=0x423006e0)
    at nsAsyncStreamListener.cpp:210
#4  0x409882b7 in nsStreamListenerEvent::HandlePLEvent (aEvent=0x423006f8)
    at nsAsyncStreamListener.cpp:97
#5  0x4012718e in PL_HandleEvent (self=0x423006f8) at plevent.c:575
#6  0x40126fac in PL_ProcessPendingEvents (self=0x8b0d9f8) at plevent.c:508
#7  0x40128df9 in nsEventQueueImpl::ProcessPendingEvents (this=0x8b0d9d0)
    at nsEventQueue.cpp:356
#8  0x40c4ea44 in ?? ()
   from /builds/sspitzer/mozilla/dist/bin/components/libwidget_gtk.so
#9  0x40c4e67f in ?? ()
   from /builds/sspitzer/mozilla/dist/bin/components/libwidget_gtk.so
#10 0x40e1552a in ?? () from /usr/lib/libglib-1.2.so.0
#11 0x40e16be6 in ?? () from /usr/lib/libglib-1.2.so.0
#12 0x40e171a1 in ?? () from /usr/lib/libglib-1.2.so.0
#13 0x40e17341 in ?? () from /usr/lib/libglib-1.2.so.0
#14 0x40d41209 in ?? () from /usr/lib/libgtk-1.2.so.0
#15 0x40c4f13a in ?? ()
   from /builds/sspitzer/mozilla/dist/bin/components/libwidget_gtk.so
#16 0x4074a3f4 in ?? ()
   from /builds/sspitzer/mozilla/dist/bin/components/libnsappshell.so
#17 0x80562e5 in main1 (argc=1, argv=0xbffffa14, nativeApp=0x0)
    at nsAppRunner.cpp:1004
#18 0x805698e in main (argc=1, argv=0xbffffa14) at nsAppRunner.cpp:1185

Comment 1

[(not reading, please use [e-mail] instead)]

possibly related to bug #52397?

Comment 2

[Darin Fisher]

Testing this under Linux BRANCH CVS build from 10/5/00, I found mozilla hitting
an XPCOM assertion in nsEventQueue.cpp:

###!!! ASSERTION: attemping to process events on the wrong thread:
'correctThread', file nsEventQueue.cpp, line 340
###!!! Break: at file nsEventQueue.cpp, line 340

I'm appending the stack trace, and sending this over to XPCOM.

Comment 3

[Darin Fisher]

Attachment: stack trace

Comment 4

[(not reading, please use [e-mail] instead)]

I wonder if this got fixed with dougt's plevent.c change?

Comment 5

[Darin Fisher]

I tested it against his new plevent.c

Comment 6

[Doug Turner (:dougt)]

Darin, is the reproducable with any of the xpinstall scripts on http://jimbob?

Comment 7

[Darin Fisher]

http://jimbob/update.html reproduces the problem.

Comment 8

[Doug Turner (:dougt)]

XPInstall is creating an native event queue on a thread that is not the UI
thread.  This is bad because native event queues get registered with the OS for
service.  

http://lxr.mozilla.org/seamonkey/source/xpinstall/src/nsSoftwareUpdateRun.cpp#369

should be CreateMonitoredThreadEventQueue.

After this change, xpinstall will be responsible for handling events that are
queued on its thread.  

Comment 9

[Doug Turner (:dougt)]

Attachment: Fixes problem

Comment 10

[Doug Turner (:dougt)]

Dan - Please review diff.  I have tested it with a couple of xpi's on jimbob
without any trouble.

Darin - can you verify that this patch indeed fixes all your woes?  Or at least
the ones with xpinstall :-)

Comment 11

[Doug Turner (:dougt)]

bouncing to dveditz.  This problem is fixed, you just need to get it reviewed
and check it in.  :-)

Comment 12

[Darin Fisher]

This patch seems to do the trick ;-)

Comment 13

[Daniel Veditz [:dveditz]]

This is a simple safe fix which will fix intermittent crashes.
r=dveditz

Comment 14

[Daniel Veditz [:dveditz]]

sr=brendan/danm received in e-mail

Comment 15

[Christopher Blizzard (:blizzard)]

sr=blizzard, too.  Doug, weren't we going to add asserts to the unix event queue
code to catch when people added non-monitored event queues to the UI queue?
It's bad mojo and this isn't the first time we've been cought with our pants down...

Comment 16

[Doug Turner (:dougt)]

Attachment: Patch which will assert if monitored event queues are created on the wrong thread.

Comment 17

[Doug Turner (:dougt)]

chris, this is what I have had in my tree for a while.  can you review it?  

Comment 18

[Phil Peterson]

PDT marking [rtm++] for the one-liner to fix the crash. The assertion should go
on the trunk.

Comment 19

[Daniel Veditz [:dveditz]]

Fix checked into trunk and branch

Comment 20

[Tom Everingham]

hmmm, I believe I am being blocked from testing this by bug 57070.  Will check 
back when it is fixed.

Comment 21

[Tom Everingham]

verified on branch:
Linux rh6 2000102009

needs verified on trunk

Comment 22

[Rodney Velasco]

verified on oct.25 linux trunk build.
Marking as verified and removing vtrunk keyword