1331036 - Provide checking that we're not spinning the event loop in states where it's not allowed (StableState, etc)

Status: NEW

(Core :: DOM: Core & HTML, defect, P3)

Description

[Randell Jesup [:jesup] (needinfo me)]

In bug 1330212, it was noted that we were spinning the event loop because a destructor called thread->Shutdown() from within StableState processing, which is not allowed (since it processes events).


We should add assertions or warnings to event-loop-spinning in states we don't want it in, even if only for Debug builds, or even if it requires whitelisting a number of known-ok uses.  This is a usually subtle landmine that's tripped many an engineer over the years - and as with this case, the event-loop-spinning is often not obvious to the person who caused it (for example, it may depend on who's holding the last ref to an object such as a thread hidden inside another object, like this one).

Worse yet, someone may change the internal implementation of an object that happens to get used from StableState (or other not-allowing-eventloop-spinning states), and suddenly other code subtly breaks or has sec issues.

If there was some simple way to set such a state on the current thread (or just MainThread), and a way to query it, some code could even be adaptive and when it can't be called directly submit a runnable to the CurrentThread to run when the event loops are allowed to spin, if the operation can be made async.

Smaug: feel free to cc anyone who might be interested

Smaug's comment from bug 1330212 comment 9:
So we would need to add some annotations on stack when spinning event loop isn't ok and then assert in thread code if someone is trying to spin the loop.
That might be useful. naAutoScriptBlocker could inherit from such "annotation class", so that we'd cover most common cases easily, and then add some more annotations for metastable and stable states.

I guess we could start from main thread only.

This might not cover all the cases, for that some rather heavy static analysis would be needed, but possibly would be useful anyhow.

Please file a bug and I'll try to find time to look at it if no one else will.
This might be useful for Quantum DOM too.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

I actually suggested this once before, and even did some work on patches (which are likely moot or ill-considered) in bug 858473

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information