Bug 312775 – ASSERTION FAILED: threadLikeAssertion.isCurrent() in WTF::assertIsCurrent in CoordinatedPlatformLayer::~CoordinatedPlatformLayer()

• Component: WPE WebKit
• Status: RESOLVED FIXED

[Fujii Hironori]

I tested with 311561@main.

./Tools/Scripts/build-webkit --wpe --release --cmakeargs=-DENABLE_ASSERTS=1
./Tools/Scripts/run-webkit-tests --wpe --release --additional-header=UseSkiaForComposition=true compositing


ASSERTION FAILED: threadLikeAssertion.isCurrent()
WTF/Headers/wtf/ThreadAssertions.h(163) : void WTF::assertIsCurrent(const ThreadLikeAssertion &)
1   0x7f5b5c765f24 WTF::RefCountDebuggerImpl<(WTF::RefCountIsThreadSafe)0>::applyRefDerefThreadingCheck(unsigned int) const
2   0x7f5b5c765e09 WTF::RefCountedBase::derefBase() const
3   0x7f5b5fe8d5d8 WebCore::CoordinatedPlatformLayer::~CoordinatedPlatformLayer()
4   0x7f5b5fe8d68e WebCore::CoordinatedPlatformLayer::~CoordinatedPlatformLayer()
5   0x7f5b5fe99ded WebCore::GraphicsLayerCoordinated::~GraphicsLayerCoordinated()
6   0x7f5b5fe99ebe WebCore::GraphicsLayerCoordinated::~GraphicsLayerCoordinated()
7   0x7f5b635a69c4 WebCore::RenderLayerCompositor::~RenderLayerCompositor()
8   0x7f5b635a6dc2 WebCore::RenderLayerCompositor::~RenderLayerCompositor()
9   0x7f5b636b12c8 WebCore::RenderView::~RenderView()
10  0x7f5b636b1522 WebCore::RenderView::~RenderView()
11  0x7f5b621070c2 WebCore::Document::destroyRenderTree()
12  0x7f5b62107b51 WebCore::Document::willBeRemovedFromFrame()
13  0x7f5b62cfe693 WebCore::LocalFrame::setView(WTF::RefPtr<WebCore::LocalFrameView, WTF::RawPtrTraits<WebCore::LocalFrameView>, WTF::DefaultRefDerefTraits<WebCore::LocalFrameView> >&&)
14  0x7f5b62d04bb0 WebCore::LocalFrame::createView(WebCore::IntSize const&, std::optional<WebCore::Color> const&, WebCore::IntSize const&, bool, WebCore::ScrollbarMode, bool, WebCore::ScrollbarMode, bool)
15  0x7f5b5d822d33 WebKit::WebLocalFrameLoaderClient::transitionToCommittedForNewPage(WebCore::LocalFrameLoaderClient::InitializingIframe)
16  0x7f5b62b0f52a WebCore::FrameLoader::transitionToCommitted(WebCore::CachedPage*)
17  0x7f5b62b0ccff WebCore::FrameLoader::commitProvisionalLoad()
18  0x7f5b62ab32de WebCore::DocumentLoader::commitIfReady()
19  0x7f5b62ac0ec6 WebCore::DocumentLoader::commitLoad(WebCore::SharedBuffer const&)
20  0x7f5b62bbfd49 WebCore::CachedRawResource::notifyClientsDataWasReceived(WebCore::SharedBuffer const&)
21  0x7f5b62bbf983 WebCore::CachedRawResource::updateBuffer(WebCore::FragmentedSharedBuffer const&)
22  0x7f5b62b92d90 WebCore::SubresourceLoader::didReceiveBuffer(WebCore::FragmentedSharedBuffer const&, long long, WebCore::DataPayloadType)
23  0x7f5b5d745aa3 WebKit::WebResourceLoader::didReceiveData(IPC::SharedBufferReference&&, unsigned long)
24  0x7f5b5c9f76e1 WebKit::WebResourceLoader::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
25  0x7f5b5d732ab6 WebKit::NetworkProcessConnection::dispatchMessage(IPC::Connection&, IPC::Decoder&)
26  0x7f5b5c9f427c WebKit::NetworkProcessConnection::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
27  0x7f5b5d04eb55 IPC::Connection::dispatchMessage(IPC::Decoder&)
28  0x7f5b5d04f06c IPC::Connection::dispatchMessage(WTF::UniqueRef<IPC::Decoder>)
29  0x7f5b5d04f49e IPC::Connection::dispatchOneIncomingMessage()
30  0x7f5b5fcc3268 WTF::RunLoop::performWork()
31  0x7f5b5fdb2ee9 WTF::RunLoop::RunLoop()::$_0::__invoke(void*)

[Fujii Hironori]

This is reproducible with the following command:

./Tools/Scripts/run-webkit-tests --wpe --release --additional-header=UseSkiaForComposition=true --no-retry --iter=1000 --exit-after-n-crash=1 compositing/geometry/fixed-position.html

[Carlos Garcia Campos]

I added a release assert to ~CoordinatedPlatformLayer to check that m_skiaTarget is always nullptr and I can't reproduce the issue

[Carlos Garcia Campos]

Tried with -DENABLE_ASSERTS=1 and still can't reproduce it

[Fujii Hironori]

Very easy to reproduce on my PC. 
If I add ASSERT(!m_skiaTarget) in ~CoordinatedPlatformLayer(), it fails.
I tested with GTK release builds with ENABLE_ASSERTS today.

./Tools/Scripts/build-webkit --gtk --release --cmakeargs=-DENABLE_ASSERTS=1
./Tools/Scripts/run-webkit-tests --gtk --release --additional-header=UseSkiaForComposition=true --no-retry --iter=1000 --exit-after-n-crash=1 compositing/geometry/fixed-position.html

[Carlos Garcia Campos]

(In reply to Fujii Hironori from comment #4)
> Very easy to reproduce on my PC. 

For some reason it doesn't happen here.

> If I add ASSERT(!m_skiaTarget) in ~CoordinatedPlatformLayer(), it fails.

I guess it fails without skia compositor too, if you assert m_target instead, right? Just to confirm this is an existing issue, not really introduced by skia compositor.

> I tested with GTK release builds with ENABLE_ASSERTS today.
> 
> ./Tools/Scripts/build-webkit --gtk --release --cmakeargs=-DENABLE_ASSERTS=1
> ./Tools/Scripts/run-webkit-tests --gtk --release
> --additional-header=UseSkiaForComposition=true --no-retry --iter=1000
> --exit-after-n-crash=1 compositing/geometry/fixed-position.html

If you can debug it, we need to understand why invalidateTarget is not called before the layer is destroyed.

[Carlos Garcia Campos]

(In reply to Fujii Hironori from comment #4)
> Very easy to reproduce on my PC. 

For some reason it doesn't happen here.

> If I add ASSERT(!m_skiaTarget) in ~CoordinatedPlatformLayer(), it fails.

I guess it fails without skia compositor too, if you assert m_target instead, right? Just to confirm this is an existing issue, not really introduced by skia compositor.

> I tested with GTK release builds with ENABLE_ASSERTS today.
> 
> ./Tools/Scripts/build-webkit --gtk --release --cmakeargs=-DENABLE_ASSERTS=1
> ./Tools/Scripts/run-webkit-tests --gtk --release
> --additional-header=UseSkiaForComposition=true --no-retry --iter=1000
> --exit-after-n-crash=1 compositing/geometry/fixed-position.html

If you can debug it, we need to understand why invalidateTarget is not called before the layer is destroyed.

[Fujii Hironori]

(In reply to Carlos Garcia Campos from comment #6)
> I guess it fails without skia compositor too, if you assert m_target
> instead, right? Just to confirm this is an existing issue, not really
> introduced by skia compositor.

That's true. ASSERT(!m_target) also fails.

[Fujii Hironori]

This patch fixes the problem.

diff --git a/Source/WebCore/platform/graphics/texmap/coordinated/GraphicsLayerCoordinated.cpp b/Source/WebCore/platform/graphics/texmap/coordinated/GraphicsLayerCoordinated.cpp
index fa6146d5c71b..d05e055a4c93 100644
--- a/Source/WebCore/platform/graphics/texmap/coordinated/GraphicsLayerCoordinated.cpp
+++ b/Source/WebCore/platform/graphics/texmap/coordinated/GraphicsLayerCoordinated.cpp
@@ -86,7 +86,6 @@ GraphicsLayerCoordinated::~GraphicsLayerCoordinated()
 {
     if (m_contentsBufferProxy)
         m_contentsBufferProxy->setTargetLayer(nullptr);
-    m_platformLayer->setOwner(nullptr);
     if (m_parent)
         downcast<GraphicsLayerCoordinated>(*m_parent).noteLayerPropertyChanged(Change::Children, ScheduleFlush::Yes);
     willBeDestroyed();


CoordinatedPlatformLayer::setOwner(nullptr) calls detachLayer.
That's the reason invalidateTarget() isn't called.

[Fujii Hironori]

Browsing some web sites with the above fix and ASSERT(!m_target), it still fails.
In this time, it's in scrolling thread. The callstack looks a bit broken maybe because I'm using release build assertion enabled.

ASSERTION FAILED: !m_target
../../../Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedPlatformLayer.cpp(77) : virtual WebCore::CoordinatedPlatformLayer::~CoordinatedPlatformLayer()
1   0x7f47c64ab937 WebCore::CoordinatedPlatformLayer::~CoordinatedPlatformLayer()
2   0x7f47c64ab95e WebCore::CoordinatedPlatformLayer::~CoordinatedPlatformLayer()
3   0x7f47c64ab543 WebCore::CoordinatedPlatformLayer::~CoordinatedPlatformLayer()
4   0x7f47c64ab95e WebCore::CoordinatedPlatformLayer::~CoordinatedPlatformLayer()
5   0x7f47c64ab543 WebCore::CoordinatedPlatformLayer::~CoordinatedPlatformLayer()
6   0x7f47c64ab95e WebCore::CoordinatedPlatformLayer::~CoordinatedPlatformLayer()
7   0x7f47c64ab543 WebCore::CoordinatedPlatformLayer::~CoordinatedPlatformLayer()
8   0x7f47c64ab95e WebCore::CoordinatedPlatformLayer::~CoordinatedPlatformLayer()
9   0x7f47c64ab543 WebCore::CoordinatedPlatformLayer::~CoordinatedPlatformLayer()
10  0x7f47c64ab95e WebCore::CoordinatedPlatformLayer::~CoordinatedPlatformLayer()
11  0x7f47c95605a1 WebCore::ScrollingTreeScrollingNode::~ScrollingTreeScrollingNode()
12  0x7f47c649fd7e WebCore::ScrollingTreeFrameScrollingNodeCoordinated::~ScrollingTreeFrameScrollingNodeCoordinated()
13  0x7f47c649d97d void WTF::ThreadSafeWeakPtrControlBlock::strongDeref<WebCore::ScrollingTreeNode, (WTF::DestructionThread)0>() const
14  0x7f47c649f0de WebCore::ScrollingTreeCoordinated::~ScrollingTreeCoordinated()
15  0x7f47c631ab3d void WTF::ThreadSafeWeakPtrControlBlock::strongDeref<WebCore::ScrollingTree, (WTF::DestructionThread)0>() const
16  0x7f47c959a77d WTF::Detail::CallableWrapper<WebCore::ThreadedScrollingCoordinator::pageDestroyed()::$_0, void>::~CallableWrapper()
17  0x7f47bf591c61 WTF::RunLoop::performWork()
18  0x7f47bf6855b9 WTF::RunLoop::RunLoop()::$_0::__invoke(void*)
19  0x7f47bf683a89 WTF::RunLoop::$_3::__invoke(_GSource*, int (*)(void*), void*)
20  0x7f47bfaa4f7f g_main_dispatch
21  0x7f47bfaa76f8 g_main_context_dispatch
22  0x7f47bf684216 WTF::RunLoop::runGLibMainLoopIteration(WTF::RunLoop::MayBlock)
23  0x7f47bf684557 WTF::RunLoop::run()
24  0x7f47bf5f5e9d WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*)
25  0x7f47bf68bad9 WTF::wtfThreadEntryPoint(void*)
26  0x7f47b99c2aa4 start_thread
27  0x7f47b9a4fa64 __clone

[Carlos Garcia Campos]

(In reply to Fujii Hironori from comment #8)
> This patch fixes the problem.
> 
> diff --git
> a/Source/WebCore/platform/graphics/texmap/coordinated/
> GraphicsLayerCoordinated.cpp
> b/Source/WebCore/platform/graphics/texmap/coordinated/
> GraphicsLayerCoordinated.cpp
> index fa6146d5c71b..d05e055a4c93 100644
> ---
> a/Source/WebCore/platform/graphics/texmap/coordinated/
> GraphicsLayerCoordinated.cpp
> +++
> b/Source/WebCore/platform/graphics/texmap/coordinated/
> GraphicsLayerCoordinated.cpp
> @@ -86,7 +86,6 @@ GraphicsLayerCoordinated::~GraphicsLayerCoordinated()
>  {
>      if (m_contentsBufferProxy)
>          m_contentsBufferProxy->setTargetLayer(nullptr);
> -    m_platformLayer->setOwner(nullptr);
>      if (m_parent)
>         
> downcast<GraphicsLayerCoordinated>(*m_parent).
> noteLayerPropertyChanged(Change::Children, ScheduleFlush::Yes);
>      willBeDestroyed();
> 
> 
> CoordinatedPlatformLayer::setOwner(nullptr) calls detachLayer.
> That's the reason invalidateTarget() isn't called.

Thanks for debugging this. So, detach layer removes the layer from m_layers, but not from committed layers. If the layer is detached before being committed, then it doesn't have a target layer for sure. Now, I think there's a problem when the last layer is destroyed... I'll check it.

[Fujii Hironori]

Created attachment 479265 [details]
debug logging patch

I applied this debug logging patch, and ran the following commands:

NUMBER_OF_PROCESSORS=50 ./Tools/Scripts/build-webkit --gtk --release --cmakeargs=-DENABLE_ASSERTS=1
./Tools/Scripts/run-webkit-tests --gtk --release --no-retry --iter=1000 --exit-after-n-crash=1 compositing/geometry/fixed-position.html

[Fujii Hironori]

Created attachment 479267 [details]
fixed-position-stderr.txt

Then, I got this log.

> CoordinatedPlatformLayer:72 0x7f2e0e500800
> (..)
> setChildren:732 0x7f2f01004000 0x7f2e0e500800
> (..)
> flushCompositingState:277 root 0x7f2f01004000
> ensureTarget:111 0x7f2f01004000
> ensureTarget:111 0x7f2e0e500800
> (..)
> ~CoordinatedPlatformLayer:78 0x7f2e0e500800
> ASSERTION FAILED: !m_target


CoordinatedPlatformLayer 0x7f2e0e500800 was created.
It was set to a child of root layer (0x7f2f01004000)
flushCompositingState for the root layer
Both root layer and the child layer did ensureTarget
The child layer was destroyed
ASSERTION FAILED

[Fujii Hironori]

Created attachment 479303 [details]
WIP patch

[Fujii Hironori]

Created attachment 479306 [details]
WIP patch

[Fujii Hironori]

Pull request: https://github.com/WebKit/WebKit/pull/63653

[EWS]

Committed 312071@main (1d5da19bdbd9): <https://commits.webkit.org/312071@main>

Reviewed commits have been landed. Closing PR #63653 and removing active labels.