Closed Bug 1928191 Opened 1 year ago Closed 1 year ago

Assertion failure: CloneScope() == StructuredCloneHolder::StructuredCloneScope::DifferentProcess, at /dom/serviceworkers/ServiceWorkerCloneData.cpp:44

Categories

(Core :: DOM: Service Workers, defect, P3)

Product:
Core
Component:
DOM: Service Workers
Platform:
x86_64
Linux
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
134 Branch
Tracking Flags:
Tracking Status
firefox-esr115 --- unaffected
firefox-esr128 --- unaffected
firefox132 --- unaffected
firefox133 --- wontfix
firefox134 --- fixed

People

(Reporter: jkratzer, Assigned: asuth)

References

(Blocks 1 open bug, Regression)

Details

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

Attachments

(2 files)

Testcase
4.36 KB, application/octet-stream
Details
Bug 1928191 - Restore ServiceWorker.postMessage eager cross-process error. r=#dom-workers!
48 bytes, text/x-phabricator-request
Details | Review

Testcase found while fuzzing mozilla-central rev 12d31a1006e7 (built with: --enable-address-sanitizer --enable-fuzzing).

Testcase can be reproduced using the following commands:

$ pip install fuzzfetch grizzly-framework --upgrade
$ python -m fuzzfetch --build 12d31a1006e7 --asan --fuzzing  -n firefox
$ python -m grizzly.replay.bugzilla ./firefox/firefox <bugid>

Assertion failure: CloneScope() == StructuredCloneHolder::StructuredCloneScope::DifferentProcess, at /dom/serviceworkers/ServiceWorkerCloneData.cpp:44

    =================================================================
    ==103948==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x7185fbdd7057 bp 0x7185dfffac70 sp 0x7185dfffaae0 T21)
    ==103948==The signal is caused by a WRITE memory access.
    ==103948==Hint: address points to the zero page.
        #0 0x7185fbdd7057 in mozilla::dom::ServiceWorkerCloneData::BuildClonedMessageData(mozilla::dom::ClonedOrErrorMessageData&) /dom/serviceworkers/ServiceWorkerCloneData.cpp:42:3
        #1 0x7185fbdd6299 in mozilla::dom::ServiceWorker::PostMessage(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Sequence<JSObject*> const&, mozilla::ErrorResult&) /dom/serviceworkers/ServiceWorker.cpp:251:14
        #2 0x7185f69ada8e in mozilla::dom::ServiceWorker_Binding::postMessage(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) /builds/worker/workspace/obj-build/dom/bindings/./ServiceWorkerBinding.cpp:315:32
        #3 0x7185f77eb624 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /dom/bindings/BindingUtils.cpp:3266:13
        #4 0x7185fe237d04 in CallJSNative /js/src/vm/Interpreter.cpp:528:13
        #5 0x7185fe237d04 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /js/src/vm/Interpreter.cpp:624:12
        #6 0x7185ff1d74f0 in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICFallbackStub*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /js/src/jit/BaselineIC.cpp:1683:10
        #7 0x377fb63149b3  ([anon:js-executable-memory]+0x29b3)
    
    AddressSanitizer can not provide additional info.
    SUMMARY: AddressSanitizer: SEGV /dom/serviceworkers/ServiceWorkerCloneData.cpp:42:3 in mozilla::dom::ServiceWorkerCloneData::BuildClonedMessageData(mozilla::dom::ClonedOrErrorMessageData&)
    Thread T21 created by T0 (Isolated Servic) here:
        #0 0x5c5ade9a8d01 in pthread_create /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:250:3
        #1 0x718612bdbfb8 in _PR_CreateThread /nsprpub/pr/src/pthreads/ptthread.c:429:10
        #2 0x718612bca12e in PR_CreateThread /nsprpub/pr/src/pthreads/ptthread.c:496:10
        #3 0x7185f1c10ee9 in nsThread::Init(nsTSubstring<char> const&) /xpcom/threads/nsThread.cpp:615:20
        #4 0x7185fb7c8793 in mozilla::dom::WorkerThread::Create(mozilla::dom::WorkerThreadFriendKey const&) /dom/workers/WorkerThread.cpp:109:7
        #5 0x7185fb73a507 in mozilla::dom::workerinternals::RuntimeService::ScheduleWorker(mozilla::dom::WorkerPrivate&) /dom/workers/RuntimeService.cpp:1332:37
        #6 0x7185fb738c7a in mozilla::dom::workerinternals::RuntimeService::RegisterWorker(mozilla::dom::WorkerPrivate&) /dom/workers/RuntimeService.cpp:1215:19
        #7 0x7185fb790084 in mozilla::dom::WorkerPrivate::Constructor(JSContext*, nsTSubstring<char16_t> const&, bool, mozilla::dom::WorkerKind, mozilla::dom::RequestCredentials, mozilla::dom::WorkerType, nsTSubstring<char16_t> const&, nsTSubstring<char> const&, mozilla::dom::WorkerLoadInfo*, mozilla::ErrorResult&, nsTString<char16_t>, std::function<void (bool)>&&, std::function<void ()>&&) /dom/workers/WorkerPrivate.cpp:2828:24
        #8 0x7185fb7db350 in mozilla::dom::RemoteWorkerChild::ExecWorkerOnMainThread(mozilla::dom::RemoteWorkerData&&) /dom/workers/remoteworkers/RemoteWorkerChild.cpp:390:41
        #9 0x7185fb80c8e7 in operator() /dom/workers/remoteworkers/RemoteWorkerChild.cpp:219:29
        #10 0x7185fb80c8e7 in mozilla::detail::RunnableFunction<mozilla::dom::RemoteWorkerChild::ExecWorker(mozilla::dom::RemoteWorkerData const&)::$_0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:548:5
        #11 0x7185f1bedc6a in mozilla::RunnableTask::Run() /xpcom/threads/TaskController.cpp:618:16
        #12 0x7185f1bd9f1e in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /xpcom/threads/TaskController.cpp:945:26
        #13 0x7185f1bd7738 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /xpcom/threads/TaskController.cpp:768:15
        #14 0x7185f1bd7d56 in mozilla::TaskController::ProcessPendingMTTask(bool) /xpcom/threads/TaskController.cpp:554:36
        #15 0x7185f1bf4f41 in operator() /xpcom/threads/TaskController.cpp:268:37
        #16 0x7185f1bf4f41 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() /xpcom/threads/nsThreadUtils.h:548:5
        #17 0x7185f1c1545f in nsThread::ProcessNextEvent(bool, bool*) /xpcom/threads/nsThread.cpp:1155:16
        #18 0x7185f1c201b8 in NS_ProcessNextEvent(nsIThread*, bool) /xpcom/threads/nsThreadUtils.cpp:480:10
        #19 0x7185f31c608e in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:85:21
        #20 0x7185f30abd54 in RunInternal /ipc/chromium/src/base/message_loop.cc:369:10
        #21 0x7185f30abd54 in RunHandler /ipc/chromium/src/base/message_loop.cc:362:3
        #22 0x7185f30abd54 in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:344:3
        #23 0x7185fc12cb09 in nsBaseAppShell::Run() /widget/nsBaseAppShell.cpp:148:27
        #24 0x7185fc2cd0ca in nsAppShell::Run() /widget/gtk/nsAppShell.cpp:469:33
        #25 0x7185fdf6d9dd in XRE_RunAppShell() /toolkit/xre/nsEmbedFunctions.cpp:651:20
        #26 0x7185f30abd54 in RunInternal /ipc/chromium/src/base/message_loop.cc:369:10
        #27 0x7185f30abd54 in RunHandler /ipc/chromium/src/base/message_loop.cc:362:3
        #28 0x7185f30abd54 in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:344:3
        #29 0x7185fdf6be8c in XRE_InitChildProcess(int, char**, XREChildData const*) /toolkit/xre/nsEmbedFunctions.cpp:586:34
        #30 0x5c5adea02779 in main /browser/app/nsBrowserApp.cpp:397:22
        #31 0x7186132171c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
        #32 0x71861321728a in __libc_start_main csu/../csu/libc-start.c:360:3
        #33 0x5c5ade92a5d8 in _start (/home/jkratzer/builds/m-c-20241030093012-fuzzing-asan-opt/firefox+0xd55d8) (BuildId: c31ea8134d878182118c90245d41ea32744ac7cd)
    
    ==103948==ABORTING
Attached file Testcase —

Perhaps, the write() was called somehow out of order and changed mStructuredCloneScope https://searchfox.org/mozilla-central/source/dom/base/StructuredCloneHolder.cpp#280 and CloneScope() behavior: https://searchfox.org/mozilla-central/source/dom/base/StructuredCloneHolder.h#240

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

Verified bug as reproducible on mozilla-central 20241030214633-55a693c2543b.
The bug appears to have been introduced in the following build range:

Start: 7936ca01a900402531a01de23474744fde9bbe1c (20241024094434)
End: 237dd85d4529b1708f8b3c04a2784925a38aa194 (20241024062740)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7936ca01a900402531a01de23474744fde9bbe1c&tochange=237dd85d4529b1708f8b3c04a2784925a38aa194

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

The test is putting an OffscreenCanvas in the transfer list which bumps the required scope to same-process from UnknownDestination:

  const stream = new TransformStream({})
  const canvas = new OffscreenCanvas(128, 16384)
  self.registration.active.postMessage(undefined,
    { 'transfer': [stream, canvas] })

I believe it was intentional in bug 1640606 that we wouldn't synchronously throw. The assertion is good since it's capturing that we're not properly handling the reality here. The main question here is whether we should continue to start with an initial unknown destination and then revise the assertion logic to instead be part of the "send an error and not the payload" or if we should initially be setting the scope to always be a different process. The reason we would want to set the unknown destination is because structured serialization is observable and if we need to fully traverse the objects and potentially detach transferrables, we would need to avoid erroring out early. I need to look into this a little more.

Conceptually it seems like this test case should be possible to trigger on the main thread even before bug 1113522 landed but this specific test case requires bug 1113522 to have landed in order for ServiceWorker instances to be non-null on workers.

Assignee: nobody → bugmail
Status: NEW → ASSIGNED

Based on comment #3, this bug contains a bisection range found by bugmon. However, the Regressed by field is still not filled.

:asuth, if possible, could you fill the Regressed by field and investigate this regression?

For more information, please visit BugBot documentation.

Flags: needinfo?(bugmail)

(see comment 4)

Flags: needinfo?(bugmail)

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

status-firefox134: --- → affected

Testcase crashes using the initial build (mozilla-central 20241030093012-12d31a1006e7) but not with tip (mozilla-central 20241115210754-6c508a387477.)

Unable to bisect testcase (End build crashes!):

Start: 12d31a1006e7d7e9f4b4199e8cb0e93bdae777d5 (20241030093012)
End: 6c508a387477e3b72db913a9e1761e9a433d06a2 (20241115210754)
BuildFlags: BuildFlags(asan=True, tsan=False, debug=False, fuzzing=True, coverage=False, valgrind=False, no_opt=False, fuzzilli=False, nyx=False)

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

Keywords: bugmon

Spec-wise, I understand the following to be going on for OffscreenCanvas:

So the current logic forcing us to same-process is really an artificial limitation introduced by Firefox. I don't see an existing enhancement bug to address that, but it is complicated by accessible MessagePorts being shippable and thereby always being potentially either same-process or cross-process. I have some imminent enhancement work in that area that potentially impacts how addressing that would work, so I'm not going to file the enhancement bug at this moment.

In terms of fixing this, it appears that in bug 1113522 I added a bad constraint for us to only convert same-process to an error if being sent from the main thread. I believe my (erroneous) rationale was that this check existed primarily as an eager optimization and that the RefMessageBodyService-related infrastructure would handle the potential worker cases, but that's only used for MessageChannel/BroadcastChannel. Absent the RefMessageBodyService being used, ServiceWorker::PostMessage can only use the DifferentProcess scope and so the main-thread check must be removed. (Also, absent support for ServiceWorkers to create dedicated Workers, conceptually ever ServiceWorker exists in its own agent cluster so there also is no way for the SameProcess situation to arise in this case other than the ServiceWorker messaging itself.)

See Also: → 1928874
Pushed by bugmail@asutherland.org: https://hg.mozilla.org/integration/autoland/rev/5140b4076b37 Restore ServiceWorker.postMessage eager cross-process error. r=dom-worker-reviewers,smaug

https://hg.mozilla.org/mozilla-central/rev/5140b4076b37

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
status-firefox134: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 134 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: