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: