1738185 - Verify the error handling for NS_ENSURE_STATE(mReady); in nsWindowMediator.cpp during late shutdown

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, task)

Description

[Jens Stutte [:jstutte]]

As part of the investigation in bug 1684441 we stumbled over some NS_ERROR_UNEXPECTED during late shutdown that seem too happen quite regularly to be really unexpected. Example from bug 1684441 comment 146:

[task 2021-10-24T16:46:55.931Z] 16:46:55     INFO - GECKO(2482) | JavaScript error: resource://gre/modules/AsyncShutdown.jsm, line 575: NotFoundError: No such JSWindowActor 'SpecialPowers'
...
[task 2021-10-24T16:47:11.612Z] 16:47:11     INFO - GECKO(2482) | JavaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 5094: NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIWindowMediator.getEnumerator]
[task 2021-10-24T16:53:02.682Z] 16:53:02     INFO - GECKO(2482) | RunWatchdog: Mainthread nested event loops during hang:
[task 2021-10-24T16:53:02.683Z] 16:53:02     INFO - GECKO(2482) |  --- (no nested event loop active)

The async shutdown blocker is receiving some unexpected error code. Looking at SessionStore.jsm it seems we want to get an enumerator but that throws NS_ERROR_UNEXPECTED. A possible cause for this is in GetEnumerator. My doubt would be then that we try to fetch an enumerator after we reached xpcom-shutdown as this sets mReady to false.

Either all (late) callers of GetEnumerator must handle well this error, making it become less unexpected, or we should just return an empty enumerator in this case and/or enforce the condition with an assert (which sounds more robust and consistent to me, but I ignore all the details, of course).

There are many other NS_ENSURE_STATE(mReady); in nsWindowMediator.cpp that might benefit from a revision in that sense, too.

Comment 1

[Jens Stutte [:jstutte]]

FWIW, in bug 672843 we already started the deprecation of the entire NS_ENSURE_* family of macros. Still just substituting them with NS_WARN_IF plus manual return is not enough here, IMHO.

Comment 2

[Jens Stutte [:jstutte]]

Attachment: Bug 1738185: Update the error handling in the mediator around mReady status. r?mccr8

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f27b70a61f59
Update the error handling in the mediator around mReady status. r=mccr8

Comment 4

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/f27b70a61f59