By looking a bit more into the pernosco session attached to this bug, I did notice that MatchPattern::Init is being called late in the xpcom shutdown (seems to be triggered by some pending Promise jobs related to loading the builtin search extension, which in that run got scheduled during the late shutdown). Under these conditions, AtomSet::Get does allocate again the static RefPtr (because the previous one was already destroyed as part of the xpcom shutdown), but it gets destroyed by ClearOnShutdown before the RefPtr gets to the caller: - https://searchfox.org/mozilla-central/rev/a8b75e4ba3f8ddf0e76b42681d0a7b7e78e67730/xpcom/base/ClearOnShutdown.cpp#19-24 This patch does: - check explicitly in MatchPattern::Init if we are get past the XPCOMShutdownFinal phase, and throws an explicit NS_ERROR_ILLEGAL_DURING_SHUTDOWN error if that is the case - change the signature for AtomSet::Get static method, to have an explicit non discardable nsresult as its result value, to track down other similar AtomSet::Get calls that may have to take this scenario into account (and to make it a bit more clear from the method signature that it can actually fail and the returned nsresult should be checked before actually using the refptr). As an additional side note: - I've looked into existing crash reports with this signature but I didn't find any (but I won't exclude that I may have not looked enough) - I've tried to reproduce conditions similar to the ones that I observed in the pernosco session (by cheating a bit and trying to force similar conditions with some temporary changes applied locally) but I wasn't able to trigger it in the exact same way Nevertheless by looking into some past bugzilla issues it looks that - it did already happen in the past that we would be still loading search extensions late in shutdown, (e.g. Bug 1620227) and so it doesn't seem totally surprising that there are other ways that we may be still in the progress. - we did fix some other crashes due to some C++ code being triggered by the js runtime late in shutdown (e.g. Bug 1663315 is one that I did notice in the middle of the bugs I looked), and so it doesn't seem that unexpected that we may still have a call to MatchPattern::Init that late in the shutdown. And so this seems to be a legit scenario to account for, even if I wasn't able to trigger it exactly like Grizzly triggered it during a fuzzy test run. Differential Revision: https://phabricator.services.mozilla.com/D112638
9.1 KiB
9.1 KiB