ScriptPromiseResolver consistency issue |
||||
Issue descriptionAnother issue that flakes every now and then on ToT and is not reproducible so far. The DCHECK seems to verify consistency of the state during destructor. Not sure what is off here. Failing test: media/crash-in-media-moved-to-newdocument.html Build: Regular linux release build with DCHECKs crash log for renderer (pid <unknown>): STDOUT: <empty> STDERR: [1:1:0410/031559.473181:FATAL:script_promise_resolver.h(57)] Check failed: state_ == kDetached || !is_promise_called_ || !GetScriptState()->ContextIsValid() || !GetExecutionContext() || GetExecutionContext()->IsContextDestroyed(). STDERR: #0 0x7ff5b0ef655d base::debug::StackTrace::StackTrace() STDERR: #1 0x7ff5b0ef4b1c base::debug::StackTrace::StackTrace() STDERR: #2 0x7ff5b0f7b92a logging::LogMessage::~LogMessage() STDERR: #3 0x7ff5a12ee5b1 blink::ScriptPromiseResolver::~ScriptPromiseResolver() STDERR: #4 0x7ff5a132e574 blink::GarbageCollectedFinalized<>::FinalizeGarbageCollectedObject() STDERR: #5 0x7ff5a132e545 blink::FinalizerTraitImpl<>::Finalize() STDERR: #6 0x7ff5a132e525 blink::FinalizerTrait<>::Finalize() STDERR: #7 0x7ff59f8aaee1 blink::HeapObjectHeader::Finalize() STDERR: #8 0x7ff59f8b21b8 blink::NormalPage::Sweep() STDERR: #9 0x7ff59f8ac189 blink::BaseArena::SweepUnsweptPage() STDERR: #10 0x7ff59f8ac744 blink::BaseArena::CompleteSweep() STDERR: #11 0x7ff59f8c5d73 blink::ThreadState::EagerSweep() STDERR: #12 0x7ff59f8c5699 blink::ThreadState::PreSweep() STDERR: #13 0x7ff59f8c525a blink::ThreadState::IncrementalMarkingFinalize() STDERR: #14 0x7ff59f8c4a03 blink::ThreadState::RunScheduledGC() STDERR: #15 0x7ff59f8c67d7 blink::ThreadState::SafePoint() STDERR: #16 0x7ff59f3bddae blink::GCTaskObserver::DidProcessTask() STDERR: #17 0x7ff59f9c9f29 blink::scheduler::WebThreadBase::TaskObserverAdapter::DidProcessTask() STDERR: #18 0x7ff59f99c30f blink::scheduler::TaskQueueManagerImpl::NotifyDidProcessTask() STDERR: #19 0x7ff59f99bd9d blink::scheduler::TaskQueueManagerImpl::DidRunTask() STDERR: #20 0x7ff59f9ad327 blink::scheduler::internal::ThreadControllerImpl::DoWork() STDERR: #21 0x7ff59f9b0091 _ZN4base8internal13FunctorTraitsIMN5blink9scheduler8internal20ThreadControllerImplEFvNS4_19SequencedTaskSource8WorkTypeEEvE6InvokeIRKNS_7WeakPtrIS5_EEJRKS7_EEEvS9_OT_DpOT0_ STDERR: #22 0x7ff59f9afff5 _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN5blink9scheduler8internal20ThreadControllerImplEFvNS6_19SequencedTaskSource8WorkTypeEERKNS_7WeakPtrIS7_EEJRKS9_EEEvOT_OT0_DpOT1_ STDERR: #23 0x7ff59f9aff6d _ZN4base8internal7InvokerINS0_9BindStateIMN5blink9scheduler8internal20ThreadControllerImplEFvNS5_19SequencedTaskSource8WorkTypeEEJNS_7WeakPtrIS6_EES8_EEEFvvEE7RunImplIRKSA_RKNSt3__15tupleIJSC_S8_EEEJLm0ELm1EEEEvOT_OT0_NSJ_16integer_sequenceImJXspT1_EEEE STDERR: #24 0x7ff59f9afe7c _ZN4base8internal7InvokerINS0_9BindStateIMN5blink9scheduler8internal20ThreadControllerImplEFvNS5_19SequencedTaskSource8WorkTypeEEJNS_7WeakPtrIS6_EES8_EEEFvvEE3RunEPNS0_13BindStateBaseE STDERR: #25 0x7ff5b0ea51ae _ZNO4base12OnceCallbackIFvvEE3RunEv STDERR: #26 0x7ff5b0efa698 base::debug::TaskAnnotator::RunTask() STDERR: #27 0x7ff5b0f9a529 base::internal::IncomingTaskQueue::RunTask() STDERR: #28 0x7ff5b0fa3947 base::MessageLoop::RunTask() STDERR: #29 0x7ff5b0fa3bb8 base::MessageLoop::DeferOrRunPendingTask() STDERR: #30 0x7ff5b0fa3ee9 base::MessageLoop::DoWork() STDERR: #31 0x7ff5b0fa5f27 base::MessagePumpDefault::Run() STDERR: #32 0x7ff5b0fa311c base::MessageLoop::Run() STDERR: #33 0x7ff5b1059e6d base::RunLoop::Run() STDERR: #34 0x7ff5af521d45 content::RendererMain() STDERR: #35 0x7ff5af95f271 content::RunZygote() STDERR: #36 0x7ff5af95ffbe content::RunNamedProcessTypeMain() STDERR: #37 0x7ff5af9624e4 content::ContentMainRunnerImpl::Run() STDERR: #38 0x7ff5af957f75 content::ContentServiceManagerMainDelegate::RunEmbedderProcess() STDERR: #39 0x7ff5a54bcdfc service_manager::Main() STDERR: #40 0x7ff5af95ebb5 content::ContentMain() STDERR: #41 0x00000061b155 main STDERR: #42 0x7ff595189f45 __libc_start_main STDERR: #43 0x00000061b02a _start STDERR:
,
Apr 26 2018
Reproduces on ToT rev: 6c2e15ce3aaf1b7eeb067fca773632c1a11aeff1 gn args: is_component_build = true is_debug = true use_goma = true symbol_level = 2 enable_blink_heap_incremental_marking = true cmd line: out/Debug/content_shell --enable-blink-features=HeapIncrementalMarkingStress --run-layout-test media/crash-in-media-moved-to-newdocument.html
,
Apr 26 2018
Alright, this is what is happening 1. HTMLMediaElement::playForBindings: creates a PromiseResolver and stores it in play_promise_resolvers_. 2. HTMLMediaElement::RejectPlayPromises: copies over play_promise_resolvers_ to play_promise_reject_list_ and clears play_promise_resolvers_ 3. HTMLMediaElement::RejectPlayPromisesInternal: calls Reject on the resolver and clears play_promise_reject_list_ That seems good. Now the Reject() part: 3a. Calls into ScriptPromiseResolver::ResolveOrReject 3b. We reach ScriptForbiddenScope::IsScriptForbidden() 3c. Because the script is forbidden we call timer_.StartOneShot(...) which returns to the event loop At the event loop we start a GC and immediately finalize. The ScriptPromiseResolver will be gone because there's nothing keeping it alive. The questionable part is: https://cs.chromium.org/chromium/src/third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h?q=ScriptPromiseResolver&dr=CSs&l=154 Kentaro, can you help triage this? This does not seem like a GC issue.
,
Apr 26 2018
Btw, a regular (idle) GC at the event loop can also happen on ToT; it's just very unlikely.
,
Apr 27 2018
+dcheng +tzik I'd say that this is a design bug of TaskRunnerTimer. 1) TaskRunnerTimer is created with a |this| pointer. Note that the |this| pointer is not wrapped with WrapPersistent. 2) timer.StartOneShot() is called. 3) The |this| object is garbage-collected. 4) The timer fires and crashes. My preference would be to deprecate TaskRunnerTimer and replace it with PostTask. Then we can use WrapPersistent. Any thoughts?
,
Apr 27 2018
I think TaskRunnerTimer doesn't cause the crash in that scenario. - If |this| is already swept and its destructor is called, timer_ is cancelled. - If |this| is not marked in a marking phase, and waiting for a lazy sweep phase, TaskRunnerTimer detects that with TimerIsObjectAliveTrait<>::IsHeapObjectAlive() and cancel the invocation.
,
Apr 27 2018
The dtor of ScriptPromiseResolver checks a few consistency conditions [1]. Maybe something is missing there? Oilpan definitelly doesn't keep the object alive, which might actually be ok considering that everything seems to be cancelled. [1] https://cs.chromium.org/chromium/src/third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h?q=ScriptPromiseResolver&dr=CSs&l=49
,
Apr 27 2018
PostTask will work fine. But, I think it's easier to just add KeepAliveWhilePending() at StartOneShot().
,
Apr 28 2018
tzik: Thanks for looking into this bug! Would you mind handling it?
,
Apr 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8c40146ad5239f58f1426b25ed58cca9a7f57017 commit 8c40146ad5239f58f1426b25ed58cca9a7f57017 Author: tzik <tzik@chromium.org> Date: Mon Apr 30 00:36:17 2018 Keep alive ScriptPromiseResolver on a delayed resolution If a ScriptPromiseResolver is requested to resolve in a ScriptForbiddenScope, it delays the resolution. However, as no one retains a reference to the SPR instance, the GC may collect it before the promise is fully resolved. Bug: 831118 Change-Id: Ic8b506d808a7d7cf851e25fe2fb6e7d3aa7464c0 Reviewed-on: https://chromium-review.googlesource.com/1034046 Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Taiju Tsuiki <tzik@chromium.org> Cr-Commit-Position: refs/heads/master@{#554684} [modify] https://crrev.com/8c40146ad5239f58f1426b25ed58cca9a7f57017/third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h [modify] https://crrev.com/8c40146ad5239f58f1426b25ed58cca9a7f57017/third_party/blink/renderer/bindings/core/v8/script_promise_resolver_test.cc
,
Apr 30 2018
|
||||
►
Sign in to add a comment |
||||
Comment 1 by peria@chromium.org
, Apr 12 2018