Closed Bug 998880 Opened 9 years ago Closed 9 years ago

nsThread shouldn't busy-loop in NS_ProcessNextEvent waiting for a thread to shutdown


(Core :: XPCOM, defect)

Not set





(Reporter: jesup, Assigned: jesup)



(Keywords: perf)


(1 file)

This bit in nsThread.cpp:
  // Process events on the current thread until we receive a shutdown ACK.
  while (!context.shutdownAck)

This busy-loops, since it doesn't let NS_ProcessNextEvent wait. (add a ', true' to allow waiting)

The same issue applies to DISPATCH_SYNC

Busy-looping here just slows down what we want to happen...
Comment on attachment 8409575 [details] [diff] [review]
don't busy-wait in nsThread waiting for shutdown or DISPATCH_SYNC

did some basic local smoketests before submitting Try
Attachment #8409575 - Flags: review?(benjamin)
So it looks like this triggers an issue in js::ctypes::ConvertToJS() and a repeatable test failure in content/chrome/js/xpconnect/tests/chrome/test_bug853571.xul

The ConvertToJS() failure in content/chrome/dom/ipc/tests/test_process_error.xul looks very interesting (since I can't think of much that *should* be sensitive to changing from busy-wait to wait)
Looks like implies the base I used simply has a perma-orange (off inbound) for the xpconnect orange
Blocks: 999016
The ConvertToJs() failure is an intentional ctypes test crash in

Per IRC with bent, we need to find out how to annotate this to ASAN to avoid it kicking on this.
Updated Try, expect ASAN failure still
Hmm, no ASAN error.  retriggering.  Maybe I just picked a poor base
Ok, it's green in the Try.  However:  the ASAN oth run still hits the asan failure - but it does on inbound, as well.  So it has nothing to do with my patch.
Comment on attachment 8409575 [details] [diff] [review]
don't busy-wait in nsThread waiting for shutdown or DISPATCH_SYNC

I checked both of these cases and they dispatch back to the main thread, so I think this is safe from deadlock. But boy is it grimy.
Attachment #8409575 - Flags: review?(benjamin) → review+
Assignee: nobody → rjesup
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.