Closed Bug 273819 Opened 20 years ago Closed 20 years ago

ASSERTION: Native event queues should only be used on the main thread

Categories

(Core :: XPCOM, defect)

defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla1.8beta1

People

(Reporter: darin.moz, Assigned: darin.moz)

Details

(Keywords: crash)

Attachments

(1 file)

NTDLL! 77f75a58()
nsDebugImpl::Assertion(nsDebugImpl * const 0x00f8d310, const char * 0x00373900,
const char * 0x003738e4, const char * 0x003738a8, int 0x000000e4) line 290
nsDebug::Assertion(const char * 0x00373900, const char * 0x003738e4, const char
* 0x003738a8, int 0x000000e4) line 109
nsEventQueueImpl::NotifyObservers(const char * 0x0033bdec
gDestroyedNotification) line 228 + 34 bytes
nsEventQueueImpl::~nsEventQueueImpl() line 139
nsEventQueueImpl::`scalar deleting destructor'(unsigned int 0x00000001) + 15 bytes
nsEventQueueImpl::Release(nsEventQueueImpl * const 0x0315f318) line 195 + 142 bytes
nsCOMPtr<nsIEventQueue>::~nsCOMPtr<nsIEventQueue>() line 584
nsTimerImpl::PostTimerEvent() line 508 + 8 bytes
TimerThread::Run(TimerThread * const 0x003e82b0) line 287
nsThread::Main(void * 0x00f90538) line 118 + 26 bytes
_PR_NativeRunThread(void * 0x00f90638) line 436 + 13 bytes
pr_root(void * 0x00f90638) line 116 + 13 bytes
_threadstartex(void * 0x00f908c0) line 212 + 13 bytes
KERNEL32! 77e7d33b()

It would appear that the last reference to this particular native event queue is
being held by the timer thread.  As a result, we destroy the native event queue
on the timer thread, and in doing so, we call into the appshell code on the
timer thread.  This explains why checking IsMainThread was "wrong" previously.

What we should do I think is move the final call to NotifyObservers from the
destructor into CheckForDeactivation since that function will only ever be
called on the main thread.

This bug is a potential crash on all platforms.
Severity: normal → major
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8beta
It actually isn't possible to tear down the event queue in CheckForDeactivation
since there may still be pending events that need to be processed.  We could
possibly tear down the native notify logic at that point, but that feels risky.

The solution is perhaps to tear down the event queue once we process the last
pending event on a deactivated event queue.  Then we should be fine provided
ProcessPendingEvents is always called ;-)
Attached patch v1 patchSplinter Review
Possible patch.  This seems to do the trick.
Attachment #168681 - Flags: review?(danm.moz)
Attachment #168681 - Flags: review?(danm.moz) → review+
Attachment #168681 - Flags: superreview?(bienvenu)
Attachment #168681 - Flags: superreview?(bienvenu) → superreview+
fixed-on-trunk

begin prayer to the talkback goddess.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: