Open Bug 1992432 Opened 11 months ago Updated 22 days ago

Assertion failure: AppShutdown::GetCurrentShutdownPhase() < ShutdownPhase::AppShutdownQM, at /dom/cache/Manager.cpp:267

Categories

(Core :: Storage: Cache API, defect, P3)

x86_64
Linux
defect

Tracking

()

Tracking Status
firefox145 --- wontfix

People

(Reporter: jkratzer, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: pernosco, regression, testcase, Whiteboard: [bugmon:bisected,confirmed])

Attachments

(1 file)

Testcase found while fuzzing mozilla-central rev 82fb3fb864d6 (built with: --enable-debug --enable-fuzzing).

Testcase can be reproduced using the following commands:

$ pip install fuzzfetch grizzly-framework --upgrade
$ python -m fuzzfetch --build 82fb3fb864d6 --debug --fuzzing  -n firefox
$ python -m grizzly.replay.bugzilla ./firefox/firefox <bugid>
Assertion failure: AppShutdown::GetCurrentShutdownPhase() < ShutdownPhase::AppShutdownQM, at /dom/cache/Manager.cpp:267

    ==1446653==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000000 (pc 0x70bb28e1400c bp 0x70bb180b9ce0 sp 0x70bb180b9c80 T1446666)
    ==1446653==The signal is caused by a WRITE memory access.
    ==1446653==Hint: address points to the zero page.
        #0 0x70bb28e1400c in MOZ_CrashSequence /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:237:3
        #1 0x70bb28e1400c in mozilla::dom::cache::Manager::Factory::AcquireCreateIfNonExistent(mozilla::SafeRefPtr<mozilla::dom::cache::ManagerId> const&) /dom/cache/Manager.cpp:266:5
        #2 0x70bb28e13d4c in mozilla::dom::cache::Manager::AcquireCreateIfNonExistent(mozilla::SafeRefPtr<mozilla::dom::cache::ManagerId> const&) /dom/cache/Manager.cpp:1676:10
        #3 0x70bb28dd654b in mozilla::dom::cache::CacheOpParent::Execute(mozilla::SafeRefPtr<mozilla::dom::cache::ManagerId> const&) /dom/cache/CacheOpParent.cpp:40:23
        #4 0x70bb28ddc5f8 in mozilla::dom::cache::CacheStorageParent::RecvPCacheOpConstructor(mozilla::dom::cache::PCacheOpParent*, mozilla::dom::cache::CacheOpArgs const&) /dom/cache/CacheStorageParent.cpp:106:10
        #5 0x70bb28e48217 in mozilla::dom::cache::PCacheStorageParent::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PCacheStorageParent.cpp:231:86
        #6 0x70bb2666e8a9 in mozilla::ipc::PBackgroundParent::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PBackgroundParent.cpp:2104:32
        #7 0x70bb2660b39e in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /ipc/glue/MessageChannel.cpp:1797:25
        #8 0x70bb26608920 in mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecycleProxy*, std::unique_ptr<IPC::Message, std::default_delete<IPC::Message>>) /ipc/glue/MessageChannel.cpp:1723:9
        #9 0x70bb26609327 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::ipc::MessageChannel::MessageTask&) /ipc/glue/MessageChannel.cpp:1512:3
        #10 0x70bb2660a309 in mozilla::ipc::MessageChannel::MessageTask::Run() /ipc/glue/MessageChannel.cpp:1614:14
        #11 0x70bb25a31a2a in nsThread::ProcessNextEvent(bool, bool*) /xpcom/threads/nsThread.cpp:1151:16
        #12 0x70bb25a3811f in NS_ProcessNextEvent(nsIThread*, bool) /xpcom/threads/nsThreadUtils.cpp:462:10
        #13 0x70bb2a87d5b6 in SpinEventLoopUntil<(mozilla::ProcessFailureBehavior)1, (lambda at /dom/quota/ActorsParent.cpp:2589:5)> /builds/worker/workspace/obj-build/dist/include/mozilla/SpinEventLoopUntil.h:176:25
        #14 0x70bb2a87d5b6 in mozilla::dom::quota::QuotaManager::Shutdown() /dom/quota/ActorsParent.cpp:2589:5
        #15 0x70bb2a8779b3 in mozilla::dom::quota::QuotaManager::ShutdownInstance() /dom/quota/ActorsParent.cpp:1914:16
        #16 0x70bb2a877880 in mozilla::dom::quota::RecvShutdownQuotaManager() /dom/quota/ActorsParent.cpp:1437:3
        #17 0x70bb2659c086 in mozilla::ipc::BackgroundParentImpl::RecvShutdownQuotaManager() /ipc/glue/BackgroundParentImpl.cpp:950:8
        #18 0x70bb26674b78 in mozilla::ipc::PBackgroundParent::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PBackgroundParent.cpp:3840:52
        #19 0x70bb2660b39e in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /ipc/glue/MessageChannel.cpp:1797:25
        #20 0x70bb26608920 in mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecycleProxy*, std::unique_ptr<IPC::Message, std::default_delete<IPC::Message>>) /ipc/glue/MessageChannel.cpp:1723:9
        #21 0x70bb26609327 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::ipc::MessageChannel::MessageTask&) /ipc/glue/MessageChannel.cpp:1512:3
        #22 0x70bb2660a309 in mozilla::ipc::MessageChannel::MessageTask::Run() /ipc/glue/MessageChannel.cpp:1614:14
        #23 0x70bb25a31a2a in nsThread::ProcessNextEvent(bool, bool*) /xpcom/threads/nsThread.cpp:1151:16
        #24 0x70bb25a3811f in NS_ProcessNextEvent(nsIThread*, bool) /xpcom/threads/nsThreadUtils.cpp:462:10
        #25 0x70bb26611e79 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:329:5
        #26 0x70bb2656b551 in RunHandler /ipc/chromium/src/base/message_loop.cc:361:3
        #27 0x70bb2656b551 in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:343:3
        #28 0x70bb25a2d68e in nsThread::ThreadFunc(void*) /xpcom/threads/nsThread.cpp:366:10
        #29 0x70bb3a970a1f in _pt_root /nsprpub/pr/src/pthreads/ptthread.c:191:3
        #30 0x70bb3b22baa3 in start_thread ./nptl/pthread_create.c:447:8
        #31 0x70bb3b2b8c6b in clone3 ./misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:78:0
    
    ==1446653==Register values:
    rax = 0x0000000000000000  rbx = 0x000070bb180b9d40  rcx = 0x000000000000010b  rdx = 0x000070bb3b393563
    rdi = 0x000070bb3b394700  rsi = 0x0000000000000000  rbp = 0x000070bb180b9ce0  rsp = 0x000070bb180b9c80
     r8 = 0x0000000000000000   r9 = 0x0000000000000003  r10 = 0x0000000000000000  r11 = 0x0000000000000293
    r12 = 0x0000000000000001  r13 = 0x000070baf003a620  r14 = 0x000070baf009ddf8  r15 = 0x000070bb180ba0c8
    UndefinedBehaviorSanitizer can not provide additional info.
    SUMMARY: UndefinedBehaviorSanitizer: SEGV (/home/jkratzer/builds/m-c-20251003092920-fuzzing-debug/libxul.so+0x772e00c) (BuildId: 5e3986c3ad160c50a6fc2a098f021670cb1e0181)
    ==1446653==ABORTING
Attached file Testcase
Attachment #9518093 - Attachment filename: testcase.html.undefined → testcase.html
Attachment #9518093 - Attachment mime type: text/plain → text/html

Verified bug as reproducible on mozilla-central 20251003092920-82fb3fb864d6.
The bug appears to have been introduced in the following build range:

Start: 33b4b029b5b6b9cc04c1a6c48c9b5d0943401f2a (20250911142049)
End: 08987ac25dd9a5c1044a301519f85c9fe4c525ed (20250912035938)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=33b4b029b5b6b9cc04c1a6c48c9b5d0943401f2a&tochange=08987ac25dd9a5c1044a301519f85c9fe4c525ed

Keywords: regression
Whiteboard: [bugmon:confirm] → [bugmon:bisected,confirmed]

Unable to reproduce bug 1992432 using build mozilla-central 20251003092920-82fb3fb864d6. Without a baseline, bugmon is unable to analyze this bug.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Keywords: bugmon

This bug has been marked as a regression. Setting status flag for Nightly to affected.

Severity: -- → S3
Priority: -- → P3

(In reply to Bugmon [:jkratzer for issues] from comment #3)

Unable to reproduce bug 1992432 using build mozilla-central 20251003092920-82fb3fb864d6. Without a baseline, bugmon is unable to analyze this bug.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Hey Artur, curious, were you able to reproduce this when you triaged, or are you still able to reproduce this? I am also not sure if the "regression" keyword is correctly flagged. Thanks.

Flags: needinfo?(aiunusov)

$ python -m grizzly.replay.bugzilla ./firefox/firefox.exe 1992432
[2025-10-27 17:49:55] Loaded Bug 1992432 from Bugzilla
[2025-10-27 17:49:55] Starting Grizzly Replay
[2025-10-27 17:49:55] Ignoring: log-limit, timeout
[2025-10-27 17:49:55] Using time limit: 30s, timeout: 45s
[2025-10-27 17:49:55] Repeat: 1, Minimum crashes: 1, Relaunch 1
[2025-10-27 17:49:55] HTTPS support requires NSS certutil.
[2025-10-27 17:49:55] Target does not support HTTPS, using HTTP
[2025-10-27 17:50:00] Running test (1/1)...
[2025-10-27 17:50:05] Unable to process minidump, minidump-stackwalk is required. https://lib.rs/crates/minidump-stackwalk
[2025-10-27 17:50:05] Processing result...
[2025-10-27 17:50:05] Result: Assertion failure: AppShutdown::GetCurrentShutdownPhase() < ShutdownPhase::AppShutdownQM, at /builds/worker/checkouts/gecko/dom/cache/Manager.cpp:267 (NO_STACK:NO_STACK)
[2025-10-27 17:50:05] Results successfully reproduced
[2025-10-27 17:50:05] Shutting down...
[2025-10-27 17:50:05] Done.

Flags: needinfo?(aiunusov)

So I think what this test case is doing is a variation of what we were seeing in https://bugzilla.mozilla.org/show_bug.cgi?id=1985543#c31 where if code defines a getter for then() that when we go to resolve the underlying JS promise we do not end up going through the CallbackObject path that checks nsIGlobalObject::IsDying so we don't notice that the global is shutting down. This allows the test case to effectively extend the lifetime of the worker and continually spam Cache API Caches.open requests all the way until shutdown at which point we hit the shutdown-phase assertion. This is made somewhat more possible because our Cache API implementation intentionally can maintain a backlog of operations that it will drain before letting the worker shutdown.

I do see that mozilla::dom::cache::CacheStorage::Open always calls mozilla::dom::cache::CacheStorage::HasStorageAccess and it will return false if mGlobal has been nulled out:

if (NS_WARN_IF(!mGlobal)) {
  return false;
}

So if we nulled that out, we should be good in this case. However, we never actually null out mGlobal outside of cycle collection; it does get traversed and unlinked but as long as we are holding a StrongWorkerRef we won't unroot the global and presumably then can't cycle-collect the CacheStorage. On the main thread we might be able to cycle-collect the CacheStorage because the global should get unrooted.

It looks like CacheStorage shutdown is not directly attached to the mozilla::dom::cache::CacheWorkerRef notify mechanism, although mozilla::dom::cache::Cache potentially is. (I still would need to page more of this back into my brain.)

At first glance, it seems like mozilla::dom::cache::CacheStorage which pre-dates GlobalTeardownObserver being extracted out of DOMEventTargetHelper could probably benefit from using that for its mGlobalObject which could let the storage checks fail. It's not immediately obvious if mozilla::dom::cache::Cache could also benefit or not given our desire to drain queued things, but conceptually it wouldn't hurt. That said, a general problem of this class of "you can get then() getters to run on dead globals" is that it is possible to attach GlobalTeardownObservers to an already-dying global. We will attach perfectly fine, but you may not get detached until the global object fully dies (which may be prevented by other state machine problems).

I am requesting a pernosco trace to confirm my theory here.

That said, :smaug, do you have any thoughts on this general meta-problem of defining a "then" getter letting JS code run in dying JS globals that might have evolved since https://bugzilla.mozilla.org/show_bug.cgi?id=1985543#c31 and your replies or given this new apparent manifestation of the problem?

Flags: needinfo?(smaug)
Flags: needinfo?(jkratzer)

Successfully recorded a pernosco session. A link to the pernosco session will be added here shortly.

A pernosco session for this bug can be found here.

Flags: needinfo?(jkratzer)

It looks like my theory from comment 7 is validated.

(In reply to Bugmon [:jkratzer for issues] from comment #9)

A pernosco session for this bug can be found here.

The trace is useful but note that the crash reason is slightly different with UniFFI being angry in its xpcom-shutdown logic and calling abort (but is still fundamentally the same situation):

(pernosco) call xpc_DumpJSStack(true,true,false)
0 observe(aSubject = "[xpconnect wrapped (nsIComponentManager, nsIServiceManager, nsIComponentRegistrar)]", aTopic = ""xpcom-shutdown"", _aData = "null") ["moz-src:///toolkit/components/uniffi-js/js/UniFFI.sys.mjs":848:12]
    this = [object Object]

That said, we do see here in this frame that the worker has already transitioned to Canceling and CacheOpChild::Recv__delete__ is resolving a dom::Promise and that is calling into content. So I think my theory is right and so it's definitely the case that the Cache API code could do better here but there's also the footgun question of dom::Promise not checking if the global is dying that is the reason for my NI to :smaug.

Testcase crashes using the initial build (mozilla-central 20251003092920-82fb3fb864d6) but not with tip (mozilla-central 20251031205128-90470f136143.)

Unable to bisect testcase (Start build didn't crash!):

Start: 82fb3fb864d696df869cadc6ac9faf9a8ebd69a7 (20251003092920)
End: 90470f1361431a99abb4526828c12b7adc2296cb (20251031205128)
BuildFlags: BuildFlags(asan=False, tsan=False, debug=True, fuzzing=True, coverage=False, valgrind=False, no_opt=False, fuzzilli=False, nyx=False, searchfox=False, afl=False)

Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Keywords: bugmon

thenables should get through callsetup and set callback is called only if the callback's global isn't dying
https://searchfox.org/firefox-main/rev/844aa5866304b0bcd8ed2f67bb6256cf4aa2d467/xpcom/base/CycleCollectedJSContext.cpp#230

Flags: needinfo?(smaug)

(In reply to Olli Pettay [:smaug][bugs@pettay.fi] from comment #12)

thenables should get through callsetup and set callback is called only if the callback's global isn't dying
https://searchfox.org/firefox-main/rev/844aa5866304b0bcd8ed2f67bb6256cf4aa2d467/xpcom/base/CycleCollectedJSContext.cpp#230

I think there's a misunderstanding. That code you link only happens if using the built-in Then() and only when we actually are then running what was passed into Then(). Here is what the test case is doing:

      Object.defineProperty(Cache.prototype, "then", {
          async get() {
              await caches.open("cache_name_3")
          }
      })

The pernosco trace shows us synchronously calling into content JS in step 9 of js::ResolvePromiseInternal (searchfox link). We only start to potentially involve a PromiseJobRunnable down in step 15 of ResolvePromiseInternal if "isBuiltinThen" (which it is not) if we call into EnqueuePromiseResolveThenableBuiltinJob. This diagram shows how we would get to the PromiseJobRunnable from ResolvePromiseInternal if we were going to), which may or may not be helpful.

Flags: needinfo?(smaug)

My meta-concern will be addressed by https://github.com/tc39/proposal-thenable-curtailment/ which has bug 2033912 as a meta bug and there currently doesn't appear to be a more targeted bug to reference. Clearing :smaug's needinfo.

As I noted in comment 6 switching to use a GlobalTeardownObserver mitigates the exact test case here but without introducing an ability to fail to attach to a global the test case can be altered to recreate the test case.

Flags: needinfo?(smaug)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: