Closed Bug 1768275 Opened 2 years ago Closed 2 years ago

Assertion failure: unwrappedPromise->state() != JS::PromiseState::Pending, at src/js/src/builtin/Promise-inl.h:30

Categories

(Core :: DOM: Animation, defect)

defect

Tracking

()

VERIFIED FIXED
102 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox100 --- unaffected
firefox101 --- unaffected
firefox102 --- verified

People

(Reporter: tsmith, Assigned: arai)

References

(Blocks 1 open bug, Regression)

Details

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

Attachments

(2 files)

Attached file testcase.html

Found while fuzzing m-c 20220504-1a63d68454d1 (--enable-debug --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch -d --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.html

Assertion failure: unwrappedPromise->state() != JS::PromiseState::Pending, at src/js/src/builtin/Promise-inl.h:30

#0 0x7f2dbac74fc9 in SetSettledPromiseIsHandled src/js/src/builtin/Promise-inl.h:30:3
#1 0x7f2dbac74fc9 in JS::SetSettledPromiseIsHandled(JSContext*, JS::Handle<JSObject*>) src/js/src/jsapi.cpp:2668:3
#2 0x7f2db80b2adf in mozilla::dom::Promise::SetSettledPromiseIsHandled() src/dom/promise/Promise.cpp:895:10
#3 0x7f2db4f532ce in mozilla::dom::Animation::Cancel(mozilla::PostRestyleMode) src/dom/animation/Animation.cpp:557:18
#4 0x7f2db55413a7 in mozilla::dom::Animation_Binding::cancel(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) /builds/worker/workspace/obj-build/dom/bindings/AnimationBinding.cpp:1057:24
#5 0x7f2db680017c in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) src/dom/bindings/BindingUtils.cpp:3271:13
#6 0x7f2dbbbdc030 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) src/js/src/vm/Interpreter.cpp:420:13
#7 0x7f2dbbbdb83a in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) src/js/src/vm/Interpreter.cpp:507:12
#8 0x7f2dbbbd2c86 in CallFromStack src/js/src/vm/Interpreter.cpp:578:10
#9 0x7f2dbbbd2c86 in Interpret(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:3314:16
#10 0x7f2dbbbc9f12 in js::RunScript(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:389:13
#11 0x7f2dbbbdb736 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) src/js/src/vm/Interpreter.cpp:539:13
#12 0x7f2dbbbdcd68 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) src/js/src/vm/Interpreter.cpp:605:8
#13 0x7f2dbab0e136 in js::CallSelfHostedFunction(JSContext*, JS::Handle<js::PropertyName*>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) src/js/src/vm/SelfHosting.cpp:1590:10
#14 0x7f2dba88d941 in AsyncFunctionResume(JSContext*, JS::Handle<js::AsyncFunctionGeneratorObject*>, ResumeKind, JS::Handle<JS::Value>) src/js/src/vm/AsyncFunction.cpp:152:8
#15 0x7f2dbaa750c8 in AsyncFunctionPromiseReactionJob src/js/src/builtin/Promise.cpp:2118:10
#16 0x7f2dbaa750c8 in PromiseReactionJob(JSContext*, unsigned int, JS::Value*) src/js/src/builtin/Promise.cpp:2176:12
#17 0x7f2dbbbdc030 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) src/js/src/vm/Interpreter.cpp:420:13
#18 0x7f2dbbbdb83a in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) src/js/src/vm/Interpreter.cpp:507:12
#19 0x7f2dbbbdcd68 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) src/js/src/vm/Interpreter.cpp:605:8
#20 0x7f2dba8b4831 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) src/js/src/vm/CallAndConstruct.cpp:117:10
#21 0x7f2db5accdbd in mozilla::dom::PromiseJobCallback::Call(mozilla::dom::BindingCallContext&, JS::Handle<JS::Value>, mozilla::ErrorResult&) /builds/worker/workspace/obj-build/dom/bindings/PromiseBinding.cpp:35:8
#22 0x7f2db37a4e95 in mozilla::dom::PromiseJobCallback::Call(mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/PromiseBinding.h:89:12
#23 0x7f2db37a4123 in Call /builds/worker/workspace/obj-build/dist/include/mozilla/dom/PromiseBinding.h:102:12
#24 0x7f2db37a4123 in mozilla::PromiseJobRunnable::Run(mozilla::AutoSlowOperation&) src/xpcom/base/CycleCollectedJSContext.cpp:213:18
#25 0x7f2db3791f98 in mozilla::CycleCollectedJSContext::PerformMicroTaskCheckPoint(bool) src/xpcom/base/CycleCollectedJSContext.cpp:674:17
#26 0x7f2db68135a7 in LeaveMicroTask /builds/worker/workspace/obj-build/dist/include/mozilla/CycleCollectedJSContext.h:243:7
#27 0x7f2db68135a7 in mozilla::dom::CallbackObject::CallSetup::~CallSetup() src/dom/bindings/CallbackObject.cpp:393:11
#28 0x7f2db5374ec2 in void mozilla::dom::Function::Call<nsCOMPtr<nsIGlobalObject> >(nsCOMPtr<nsIGlobalObject> const&, nsTArray<JS::Value> const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/FunctionBinding.h:72:3
#29 0x7f2db5374c04 in mozilla::dom::CallbackTimeoutHandler::Call(char const*) src/dom/base/TimeoutHandler.cpp:167:29
#30 0x7f2db50555c2 in nsGlobalWindowInner::RunTimeoutHandler(mozilla::dom::Timeout*, nsIScriptContext*) src/dom/base/nsGlobalWindowInner.cpp:6420:38
#31 0x7f2db538523a in mozilla::dom::TimeoutManager::RunTimeout(mozilla::TimeStamp const&, mozilla::TimeStamp const&, bool) src/dom/base/TimeoutManager.cpp:903:44
#32 0x7f2db5372760 in mozilla::dom::TimeoutExecutor::MaybeExecute() src/dom/base/TimeoutExecutor.cpp:179:11
#33 0x7f2db5372d09 in Notify src/dom/base/TimeoutExecutor.cpp:246:5
#34 0x7f2db5372d09 in non-virtual thunk to mozilla::dom::TimeoutExecutor::Notify(nsITimer*) src/dom/base/TimeoutExecutor.cpp
#35 0x7f2db38db75c in operator() src/xpcom/threads/nsTimerImpl.cpp:656:44
#36 0x7f2db38db75c in matchN<mozilla::Variant<nsTimerImpl::UnknownCallback, nsCOMPtr<nsITimerCallback>, nsCOMPtr<nsIObserver>, nsTimerImpl::FuncCallback, nsTimerImpl::ClosureCallback> &, (lambda at src/xpcom/threads/nsTimerImpl.cpp:656:7), (lambda at src/xpcom/threads/nsTimerImpl.cpp:657:7), (lambda at src/xpcom/threads/nsTimerImpl.cpp:660:7), (lambda at src/xpcom/threads/nsTimerImpl.cpp:661:7)> /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:309:16
#37 0x7f2db38db75c in matchN<mozilla::Variant<nsTimerImpl::UnknownCallback, nsCOMPtr<nsITimerCallback>, nsCOMPtr<nsIObserver>, nsTimerImpl::FuncCallback, nsTimerImpl::ClosureCallback> &, (lambda at src/xpcom/threads/nsTimerImpl.cpp:655:7), (lambda at src/xpcom/threads/nsTimerImpl.cpp:656:7), (lambda at src/xpcom/threads/nsTimerImpl.cpp:657:7), (lambda at src/xpcom/threads/nsTimerImpl.cpp:660:7), (lambda at src/xpcom/threads/nsTimerImpl.cpp:661:7)> /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:318:14
#38 0x7f2db38db75c in matchN<mozilla::Variant<nsTimerImpl::UnknownCallback, nsCOMPtr<nsITimerCallback>, nsCOMPtr<nsIObserver>, nsTimerImpl::FuncCallback, nsTimerImpl::ClosureCallback> &, (lambda at src/xpcom/threads/nsTimerImpl.cpp:655:7), (lambda at src/xpcom/threads/nsTimerImpl.cpp:656:7), (lambda at src/xpcom/threads/nsTimerImpl.cpp:657:7), (lambda at src/xpcom/threads/nsTimerImpl.cpp:660:7), (lambda at src/xpcom/threads/nsTimerImpl.cpp:661:7)> /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:902:12
#39 0x7f2db38db75c in match<(lambda at src/xpcom/threads/nsTimerImpl.cpp:655:7), (lambda at src/xpcom/threads/nsTimerImpl.cpp:656:7), (lambda at src/xpcom/threads/nsTimerImpl.cpp:657:7), (lambda at src/xpcom/threads/nsTimerImpl.cpp:660:7), (lambda at src/xpcom/threads/nsTimerImpl.cpp:661:7)> /builds/worker/workspace/obj-build/dist/include/mozilla/Variant.h:857:12
#40 0x7f2db38db75c in nsTimerImpl::Fire(int) src/xpcom/threads/nsTimerImpl.cpp:654:22
#41 0x7f2db38ad14e in nsTimerEvent::Run() src/xpcom/threads/TimerThread.cpp:263:11
#42 0x7f2db38cc1fd in mozilla::ThrottledEventQueue::Inner::ExecuteRunnable() src/xpcom/threads/ThrottledEventQueue.cpp:254:22
#43 0x7f2db38c8b61 in mozilla::ThrottledEventQueue::Inner::Executor::Run() src/xpcom/threads/ThrottledEventQueue.cpp:81:15
#44 0x7f2db38c9bbe in mozilla::RunnableTask::Run() src/xpcom/threads/TaskController.cpp:467:16
#45 0x7f2db38a4673 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:780:26
#46 0x7f2db38a3223 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:612:15
#47 0x7f2db38a3493 in mozilla::TaskController::ProcessPendingMTTask(bool) src/xpcom/threads/TaskController.cpp:390:36
#48 0x7f2db38cd339 in operator() src/xpcom/threads/TaskController.cpp:127:37
#49 0x7f2db38cd339 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_1>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:531:5
#50 0x7f2db38b8edf in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1180:16
#51 0x7f2db38bf41d in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:465:10
#52 0x7f2db4461904 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:107:5
#53 0x7f2db438b9d7 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:380:10
#54 0x7f2db438b8e2 in RunHandler src/ipc/chromium/src/base/message_loop.cc:373:3
#55 0x7f2db438b8e2 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:355:3
#56 0x7f2db8506178 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
#57 0x7f2dba62ee4b in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:874:20
#58 0x7f2db446284a in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:235:9
#59 0x7f2db438b9d7 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:380:10
#60 0x7f2db438b8e2 in RunHandler src/ipc/chromium/src/base/message_loop.cc:373:3
#61 0x7f2db438b8e2 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:355:3
#62 0x7f2dba62e46c in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:733:34
#63 0x55af83485e30 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
#64 0x55af83485e30 in main src/browser/app/nsBrowserApp.cpp:327:18
#65 0x7f2dc9ce20b2 in __libc_start_main /build/glibc-sMfBJT/glibc-2.31/csu/../csu/libc-start.c:308:16
#66 0x55af8345bbdc in _start (/home/worker/builds/m-c-20220504093759-fuzzing-debug/firefox-bin+0x15bdc) (BuildId: f2284d470eae54e27a7e66a4a3331675a4cf1b4e)
Flags: in-testsuite?

A Pernosco session is available here: https://pernos.co/debug/1XXQTjEXN8sMj-XzB9FTxQ/index.html

Component: DOM: Core & HTML → DOM: Animation

Bugmon Analysis
Verified bug as reproducible on mozilla-central 20220506222931-d6ef5a49cd7d.
The bug appears to have been introduced in the following build range:

Start: a8ce86cf670af93083642b33789080c463c351fe (20220502213947)
End: 8b761ddca934ee67a4e981f2cf95545d813d01fa (20220502120234)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a8ce86cf670af93083642b33789080c463c351fe&tochange=8b761ddca934ee67a4e981f2cf95545d813d01fa

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

Could this be a regression from bug 1767087?

Flags: needinfo?(arai.unmht)

yes

Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Regressed by: 1767087

Set release status flags based on info from the regressing bug 1767087

The issue here is the following:

Animation.mFinished is created lazily with pending state, when finished property is accessed:

https://searchfox.org/mozilla-central/rev/5f88c1d6977e03e22d3420d0cdf8ad0113c2eb31/dom/animation/Animation.cpp#527-530

Promise* Animation::GetFinished(ErrorResult& aRv) {
  nsCOMPtr<nsIGlobalObject> global = GetOwnerGlobal();
  if (!mFinished && global) {
    mFinished = Promise::Create(global, aRv);  // Lazily create on demand

https://drafts.csswg.org/web-animations-1/#the-current-finished-promise

4.4.11. The current finished promise

Each animation has a current finished promise. The current finished promise is
initially a pending Promise object.

And when Animation.prototype.finish is called, it's resolved with the animation itself:

In normal situation, Animation instance is not "thenable", and resolving with it immediately makes mFinished settled with the animation itself.

https://searchfox.org/mozilla-central/rev/5f88c1d6977e03e22d3420d0cdf8ad0113c2eb31/dom/animation/Animation.cpp#1845-1847

void Animation::MaybeResolveFinishedPromise() {
  if (mFinished) {
    mFinished->MaybeResolve(this);

in the following call stack:

0 Animation::MaybeResolveFinishedPromise
1 Animation::DoFinishNotificationImmediately
2 Animation::DoFinishNotification
3 Animation::UpdateFinishedState
4 Animation::UpdateTiming
5 Animation::Finish
6 Animation_Binding::finish

This maps to the following spec steps:

https://drafts.csswg.org/web-animations-1/#dom-animation-finish

void finish()
  Seeks the animation to the associated effect end in the current direction
  by running the finish an animation procedure for this object.

https://drafts.csswg.org/web-animations-1/#finish-an-animation

finish an animation for animation defined below:
...
  8. Run the procedure to update an animation’s finished state for animation
     with the did seek flag set to true, and the synchronously notify flag set
     to true.

https://drafts.csswg.org/web-animations-1/#update-an-animations-finished-state

The procedure to update an animation’s finished state for animation ... is as
follows:
...
  5. If current finished state is true and the current finished promise is not
     yet resolved, perform the following steps:
    1. Let finish notification steps refer to the following procedure:
...
      2. Resolve animation’s current finished promise object with animation.
...
    2. If synchronously notify is true, cancel any queued microtask to run the
       finish notification steps for this animation, and run the finish
       notification steps immediately.

But in the testcase, the Animation instance has then property with function value, that makes the instance "thenable", and the above MaybeResolve calls the function. And given the function doesn't do anything, mFinished is kept pending.

  a.then = async () => { }

Then, when Animation.prototype.cancel is called, Animation::Cancel calls MaybeReject on mFinished.
Before bug 1767087 patch, this rejects mFinished regardless of whether MaybeResolve is called on it or not, and mFinished is settled.
So SetSettledPromiseIsHandled works.

After bug 1767087 patch, this MaybeReject does nothing because MaybeResolve is already called on it,
and mFinished is still pending, and SetSettledPromiseIsHandled crashed with the assertion failure.

https://searchfox.org/mozilla-central/rev/5f88c1d6977e03e22d3420d0cdf8ad0113c2eb31/dom/animation/Animation.cpp#543-553

void Animation::Cancel(PostRestyleMode aPostRestyle) {
...
    if (mFinished) {
      mFinished->MaybeReject(NS_ERROR_DOM_ABORT_ERR);
      mFinished->SetSettledPromiseIsHandled();

https://drafts.csswg.org/web-animations-1/#dom-animation-cancel

void cancel()
  Clears all effects caused by this animation and aborts its playback by
  running the cancel an animation procedure for this object.

https://drafts.csswg.org/web-animations-1/#cancel-an-animation

The procedure to cancel an animation for animation is as follows:

  1. If animation’s play state is not idle, perform the following steps:
...
    2. Reject the current finished promise with a DOMException named
       "AbortError".

Simple workaround is to call SetAnyPromiseIsHandled instead of SetSettledPromiseIsHandled.

It would be better checking other callsites as well.
in most case, SetSettledPromiseIsHandled is called immediately after MaybeReject,
but if the same promise can also be resolved with random value, that does not work and it needs SetAnyPromiseIsHandled instead.

To avoid this kind of issue in the future, it might make sense to remove SetSettledPromiseIsHandled and always call SetAnyPromiseIsHandled.
the difference between them is only about whether to check the state or not.
and having extra branch there won't affect the performance much.
https://searchfox.org/mozilla-central/rev/5f88c1d6977e03e22d3420d0cdf8ad0113c2eb31/js/src/builtin/Promise-inl.h#28-41

Flags: needinfo?(arai.unmht)
Has Regression Range: --- → yes
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/autoland/rev/20e8a5b10f18
Use SetAnyPromiseIsHandled instead of SetSettledPromiseIsHandled for promise that's not guaranteed to be not-yet-resolved. r=birtles
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/34030 for changes under testing/web-platform/tests
Whiteboard: [bugmon:bisected,confirmed] → [bugmon:bisected,confirmed], [wptsync upstream]

Backed out changeset for causing linting failures on cancel.html with setTimeout error.

Push with failures

Failure log

Backout link

[task 2022-05-11T13:48:57.715Z] Creating default state directory: /builds/worker/.mozbuild
[task 2022-05-11T13:48:57.793Z] 13:48:57.793 wpt (51) | Finished in 0.07 seconds
[task 2022-05-11T13:48:57.794Z] 13:48:57.794 wpt (53) | Finished in 0.07 seconds
[task 2022-05-11T13:48:57.795Z] 13:48:57.794 wpt (50) | Finished in 0.07 seconds
[task 2022-05-11T13:48:57.819Z] 13:48:57.818 wpt (52) | Passing the following paths:
[task 2022-05-11T13:48:57.819Z] /builds/worker/checkouts/gecko/testing/web-platform/tests
[task 2022-05-11T13:48:57.820Z] No specific files specified, running the full wpt lint (this is slow)
[task 2022-05-11T13:48:57.820Z] 13:48:57.820 wpt (52) | Command: python3 /builds/worker/checkouts/gecko/testing/web-platform/tests/wpt lint --json --all
[task 2022-05-11T13:50:06.216Z] 13:50:06.216 wpt (52) | Finished in 68.49 seconds
[task 2022-05-11T13:50:06.225Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/web-platform/tests/web-animations/interfaces/Animation/cancel.html:106 | setTimeout used (SET TIMEOUT)
[task 2022-05-11T13:50:06.225Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/tools/lint/wpt.yml:0 | Lint process exited with return code 1 (wpt)
[taskcluster 2022-05-11 13:50:07.713Z] === Task Finished ===
[taskcluster 2022-05-11 13:50:08.295Z] Unsuccessful task run with exit code: 1 completed in 635.304 seconds
Flags: needinfo?(arai.unmht)
Upstream PR was closed without merging
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/autoland/rev/7d6f346527fd
Use SetAnyPromiseIsHandled instead of SetSettledPromiseIsHandled for promise that's not guaranteed to be not-yet-resolved. r=birtles
Flags: needinfo?(arai.unmht)
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 102 Branch
Upstream PR merged by moz-wptsync-bot

Bugmon Analysis
Verified bug as fixed on rev mozilla-central 20220511214930-ce64ea6b6488.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Status: RESOLVED → VERIFIED
Keywords: bugmon
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: