Closed Bug 1612928 Opened 4 years ago Closed 3 years ago

Assertion failure: !mSyncLoopTarget, at /builds/worker/workspace/build/src/dom/xhr/XMLHttpRequestWorker.cpp:839

Categories

(Core :: DOM: Networking, defect, P2)

defect

Tracking

()

RESOLVED FIXED
86 Branch
Tracking Status
firefox-esr78 --- wontfix
firefox74 --- wontfix
firefox77 --- wontfix
firefox78 --- wontfix
firefox79 --- wontfix
firefox80 --- wontfix
firefox81 --- wontfix
firefox82 --- wontfix
firefox83 --- wontfix
firefox85 --- wontfix
firefox86 --- fixed

People

(Reporter: jkratzer, Assigned: jstutte)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase, Whiteboard: [fuzzblocker][necko-triaged][bugmon:bisected,confirmed])

Attachments

(3 files)

Attached file testcase.html

Testcase found while fuzzing mozilla-central rev f4e3917a0fa1.

Assertion failure: !mSyncLoopTarget, at /builds/worker/workspace/build/src/dom/xhr/XMLHttpRequestWorker.cpp:839

rax = 0x000056118938a340   rdx = 0x0000000000000000
rcx = 0x00007ffbc707ddca   rbx = 0x00007ffba3004c90
rsi = 0x00007ffbd2b358b0   rdi = 0x00007ffbd2b34680
rbp = 0x00007ffce720a7d0   rsp = 0x00007ffce720a780
r8 = 0x00007ffbd2b358b0    r9 = 0x00007ffbd3c9c780
r10 = 0x0000000000000000   r11 = 0x0000000000000000
r12 = 0x0000000000000000   r13 = 0x00007ffce720a788
r14 = 0x00007ffce720a808   r15 = 0x00007ffba2e25e80
rip = 0x00007ffbc31c821c
OS|Linux|0.0.0 Linux 5.3.0-28-generic #30~18.04.1-Ubuntu SMP Fri Jan 17 06:14:09 UTC 2020 x86_64
CPU|amd64|family 6 model 94 stepping 3|8
GPU|||
Crash|SIGSEGV|0x0|0
0|0|libxul.so|mozilla::dom::Proxy::Teardown(bool)|hg:hg.mozilla.org/mozilla-central:dom/xhr/XMLHttpRequestWorker.cpp:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|816|0x0
0|1|libxul.so|AsyncTeardownRunnable::Run|hg:hg.mozilla.org/mozilla-central:dom/xhr/XMLHttpRequestWorker.cpp:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|350|0x13
0|2|libxul.so|mozilla::ThrottledEventQueue::Inner::ExecuteRunnable()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/ThrottledEventQueue.cpp:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|252|0x12
0|3|libxul.so|mozilla::ThrottledEventQueue::Inner::Executor::Run()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/ThrottledEventQueue.cpp:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|80|0xd
0|4|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|1220|0xe
0|5|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|486|0x11
0|6|libxul.so|mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|87|0xa
0|7|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|315|0x19
0|8|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|290|0x8
0|9|libxul.so|nsBaseAppShell::Run()|hg:hg.mozilla.org/mozilla-central:widget/nsBaseAppShell.cpp:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|137|0xd
0|10|libxul.so|nsAppStartup::Run()|hg:hg.mozilla.org/mozilla-central:toolkit/components/startup/nsAppStartup.cpp:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|272|0x10
0|11|libxul.so|XREMain::XRE_mainRun()|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsAppRunner.cpp:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|4560|0x16
0|12|libxul.so|XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&)|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsAppRunner.cpp:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|4695|0x8
0|13|libxul.so|XRE_main(int, char**, mozilla::BootstrapConfig const&)|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsAppRunner.cpp:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|4746|0x5
0|14|firefox-bin|do_main|hg:hg.mozilla.org/mozilla-central:browser/app/nsBrowserApp.cpp:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|217|0x26
0|15|firefox-bin|main|hg:hg.mozilla.org/mozilla-central:browser/app/nsBrowserApp.cpp:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|339|0xf
0|16|libc-2.27.so||||0x21b97
0|17|firefox-bin|__cxa_throw_bad_array_new_length|hg:hg.mozilla.org/mozilla-central:build/unix/stdc++compat/stdc++compat.cpp:f4e3917a0fa15fb009e0ec4403ea7d066ee0e3c0|82|0x12
0|18|firefox-bin||||0x10e30
0|19|ld-2.27.so||||0x10733
0|20|libdl-2.27.so||||0x202d80
0|21|libpthread-2.27.so||||0x219bb0
0|22|firefox-bin||||0x10e30
0|23|firefox-bin|_start|||0x29
Flags: in-testsuite?
Attached file worker.js
Whiteboard: [fuzzblocker]
Priority: -- → P2
Whiteboard: [fuzzblocker] → [fuzzblocker][necko-triaged]
Whiteboard: [fuzzblocker][necko-triaged] → [fuzzblocker][necko-triaged][bugmon:bisected,confirmed]
Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20200629154604-31fb4a2a6912.
Failed to bisect testcase (Start build crashes!):
> Start: 501f50ccafdfd30a6bc8d85f2be8704ca335d65f (20190701095159)
> End: 6d46db840d6e71ffa2c416ae018e27e921eac6e0 (20200203040425)
> BuildFlags: BuildFlags(asan=False, tsan=False, debug=True, fuzzing=False, coverage=False, valgrind=False)

Jens, the fuzzers have been tripping over this fuzzblocker for a while now. Is anyone available to have a look?

Hmm, the crash points to a MOZ_CRASH("We're going to hang at shutdown anyways.");, which looks questionable to me.

Actually I'd like to see us hanging with clear STR.

Flags: needinfo?(jstutte)
Keywords: leave-open
Assignee: nobody → jstutte
Attachment #9187679 - Attachment description: Bug 1612928: Ensure mSyncLoopTarget is null at the end of Teardown to avoid assertion and change some MOZ_CRASH to more graceful behavior. → Bug 1612928: Ensure mSyncLoopTarget is null at the end of Teardown to avoid assertion and change some MOZ_CRASH to more graceful behavior. r=#dom-workers-and-storage-reviewers
Status: NEW → ASSIGNED
Attachment #9124107 - Attachment mime type: application/x-javascript → text/plain
Pushed by rmaries@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ad36a79133a3
Ensure mSyncLoopTarget is null at the end of Teardown to avoid assertion and change some MOZ_CRASH to more graceful behavior. r=dom-workers-and-storage-reviewers,asuth

:jens, I think you actually fixed the underlying problem here by my analysis in https://phabricator.services.mozilla.com/D97010#3247133 so I think this can probably be marked fixed (in Firefox 86 per the commit)? That is, I don't think we're actually expecting any new crash signatures from the patch as it landed. (In theory the MOZ_ALWAYS_TRUE(runnable->Dispatch()) could crash via MOZ_DIAGNOSTIC_ASSERT, but I'm not seeing any crashes in XMLHttpRequestWorker against version >=86 and I'm likewise not seeing any "moz crash reason" with Dispatch in it involving XHR.)

Flags: needinfo?(jstutte)

Jason, can you confirm this through the test-case ?
Andrew, should we create a regression test from that test case then?

Flags: needinfo?(jstutte)
Flags: needinfo?(jkratzer)
Flags: needinfo?(bugmail)
Flags: needinfo?(jkratzer)
Whiteboard: [fuzzblocker][necko-triaged][bugmon:bisected,confirmed] → [fuzzblocker][necko-triaged][bugmon:bisected,confirmed,confirm]
Whiteboard: [fuzzblocker][necko-triaged][bugmon:bisected,confirmed,confirm] → [fuzzblocker][necko-triaged][bugmon:bisected,confirm]

This issue was last reported by fuzzers running m-c 20201217-2ab4142f19bc. Prior to that it was hit frequently. I don't see any obvious variations.

This particular scenario was deterministic and so a deterministic test can be landed. That said, there's the meta question of whether it makes sense to land specific test cases that the fuzzer already reliably generates coverage for (hence this having been a fuzzblocker.)

The linearized deterministic worker logic then becomes:

self.onmessage = function (e) {
  const xhr = new XMLHttpRequest()
  xhr.open('G', '', false)
  xhr.send(undefined)
  // TODO: wrap this in a catch as it will throw.
  xhr.send(undefined)
  // TODO: postMessage back to the parent to indicate the test is complete.
}

and the worker creation from the document no longer needs to attempt to reload the worker, and would become:

        const worker = new Worker('worker.js')
        worker.postMessage([], [])
// TODO: Await a postMessage from the worker following the 2nd call to send before declaring the test over.
Flags: needinfo?(bugmail)

Bugmon Analysis:
The bug appears to have been fixed in the following build range:

Start: 28b7a2b995c32e55107c8b41722396bbbe219565 (20201217150734)
End: ad36a79133a39117d6982cbd2e948b55a9ae0675 (20201217150757)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=28b7a2b995c32e55107c8b41722396bbbe219565&tochange=ad36a79133a39117d6982cbd2e948b55a9ae0675
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Keywords: bugmon
Whiteboard: [fuzzblocker][necko-triaged][bugmon:bisected,confirm] → [fuzzblocker][necko-triaged][bugmon:bisected,confirmed]

Assuming then from comment 11, comment 12 and comment 13 that we have confidence that the issue has been fixed and that fuzzers will rediscover it in case of regressions (as Andrew already stated in the testing exception comment in the patch).

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Keywords: leave-open
Resolution: --- → FIXED
See Also: → 1752863
You need to log in before you can comment on or make changes to this bug.