Oilpan GC may let scheduler access LocalFrame that's already detached |
|||
Issue descriptionOriginal issue discussed at https://chromium-review.googlesource.com/c/chromium/src/+/923688 The Patch Set 12 gave the following crash: Received signal 11 SEGV_MAPERR 000000000010 0 Chromium Framework 0x000000011ec4b9cc base::debug::StackTrace::StackTrace(unsigned long) + 28 1 Chromium Framework 0x000000011ec4b801 base::debug::(anonymous namespace)::StackDumpSignalHandler(int, __siginfo*, void*) + 2401 2 libsystem_platform.dylib 0x00007fff76ddff5a _sigtramp + 26 3 ??? 0x00007fb43f522b08 0x0 + 140412133190408 4 Chromium Framework 0x00000001233340cc blink::WebLocalFrameImpl::LocalRoot() + 28 5 Chromium Framework 0x00000001239da1b4 blink::ChromeClientImpl::GetWebLayerTreeView(blink::LocalFrame*) + 20 6 Chromium Framework 0x00000001239fbdf7 blink::Page::RequestBeginMainFrameNotExpected(bool) + 183 7 Chromium Framework 0x000000011e47ccf0 blink::scheduler::RendererSchedulerImpl::OnPendingTasksChanged(bool) + 464 8 Chromium Framework 0x000000011e45a813 blink::scheduler::IdleHelper::OnIdleTaskPostedOnMainThread() + 83 9 Chromium Framework 0x000000011e45c5b2 blink::scheduler::IdleHelper::OnIdleTaskPosted() + 162 10 Chromium Framework 0x000000011e45f397 blink::scheduler::SingleThreadIdleTaskRunner::PostNonNestableIdleTask(base::Location const&, base::OnceCallback<void (base::TimeTicks)>) + 39 11 Chromium Framework 0x000000011e4603fa blink::scheduler::WebSchedulerImpl::PostNonNestableIdleTask(base::Location const&, base::OnceCallback<void (double)>) + 346 12 Chromium Framework 0x000000011e3b58b0 blink::ThreadState::ScheduleIdleGC() + 192 13 Chromium Framework 0x000000011e3b65e7 blink::ThreadState::ScheduleGCIfNeeded() + 999 14 Chromium Framework 0x000000011e3abbbf blink::NormalPageArena::OutOfLineAllocate(unsigned long, unsigned long) + 431 15 Chromium Framework 0x000000011e3cfd49 blink::ThreadHeap::AllocateOnArenaIndex(blink::ThreadState*, unsigned long, int, unsigned long, char const*) + 313 16 Chromium Framework 0x0000000122673aa0 blink::RemoteWindowProxy::Create(v8::Isolate*, blink::RemoteFrame&, scoped_refptr<blink::DOMWrapperWorld>) + 80 17 Chromium Framework 0x000000012267390f blink::WindowProxyManager::CreateWindowProxy(blink::DOMWrapperWorld&) + 111 18 Chromium Framework 0x00000001226737cf blink::WindowProxyManager::WindowProxyManager(blink::Frame&, blink::WindowProxyManager::FrameType) + 175 19 Chromium Framework 0x00000001232f2fb8 blink::RemoteFrame::RemoteFrame(blink::RemoteFrameClient*, blink::Page&, blink::FrameOwner*) + 120 20 Chromium Framework 0x00000001232f2424 blink::RemoteFrame::Create(blink::RemoteFrameClient*, blink::Page&, blink::FrameOwner*) + 100 21 Chromium Framework 0x0000000123207bb6 blink::WebRemoteFrameImpl::InitializeCoreFrame(blink::Page&, blink::FrameOwner*, WTF::AtomicString const&) + 22 22 Chromium Framework 0x00000001231ed5f6 blink::WebFrame::Swap(blink::WebFrame*) + 1062 23 Chromium Framework 0x00000001247297ec content::RenderFrameImpl::OnSwapOut(int, bool, content::FrameReplicationState const&) + 380 <...> dcheng explained: OK I think I understand the crash. It is rather tricky. What is happening is this: 1) //content is swapping a local frame (old local frame) to a remote frame (new remote frame). 2) At this point, Page::MainFrame() still points to old local frame. 3) WebFrame::Swap() calls Detach() on the old local frame. 4) WebFrame::Swap() initializes the core frame of the new remote frame. 5) In the middle of allocating objects for the new remote frame, Oilpan schedules an idle GC. 6) This causes the scheduler to notify all WebViewSchedulers: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc?rcl=ad6860b020ad4ecfa42c78cc31217954bc24fe2a&l=2241 7) This calls into Page::RequestBeginMainFrameNotExpected. 8) Which calls into ChromeClientImpl::GetWebLayerTreeView() with the old local frame. 9) Converting LocalFrame -> WebLocalFrame returns nullptr because the LocalFrame is already detached (doing this lookup requires that Frame::Client() not be null--but old local frame is already detached) It seems weird (and a little dangerous) that this work is triggered in the middle of allocation. I am not sure how hard it is to change.
,
Mar 13 2018
,
Mar 13 2018
Would it be reasonable to avoid calling OnIdleTaskPostedOnMainThread synchronously here and just always post a task? It seems like there are other IdleHelper::Delegate::OnPendingTasksChanged methods that could be invoked, and it seems like it'd be safer to minimize the amount of code that runs in the middle of allocations.
,
Mar 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1bfa23fdee284c09f24c0ea1ec805939a96daa27 commit 1bfa23fdee284c09f24c0ea1ec805939a96daa27 Author: Alexander Timin <altimin@chromium.org> Date: Fri Mar 16 13:30:38 2018 [scheduler] Issue RequestBeginMainFrameNotExpected call asynchronously. Post a control task to call RequestBeginMainFrameNotExpected asynchronously to avoid calling it when frame tree is dirty. R=yutak@chromium.org,dcheng@chromium.org,haraken@chromium.org BUG= 820893 Change-Id: I375aa46180f20f053c13563df910d3813c22938f Reviewed-on: https://chromium-review.googlesource.com/962787 Reviewed-by: Yuta Kitamura <yutak@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Alexander Timin <altimin@chromium.org> Cr-Commit-Position: refs/heads/master@{#543683} [modify] https://crrev.com/1bfa23fdee284c09f24c0ea1ec805939a96daa27/third_party/WebKit/Source/core/page/Page.cpp [modify] https://crrev.com/1bfa23fdee284c09f24c0ea1ec805939a96daa27/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc [modify] https://crrev.com/1bfa23fdee284c09f24c0ea1ec805939a96daa27/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.h [modify] https://crrev.com/1bfa23fdee284c09f24c0ea1ec805939a96daa27/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl_unittest.cc
,
Mar 21 2018
|
|||
►
Sign in to add a comment |
|||
Comment 1 by skyos...@chromium.org
, Mar 13 2018