Bug 84278 – [Qt] ASSERT(m_thread == currentThread()) on Mac when threaded rendering is enabled for the Qt scenegraph

• Component: Platform
• Status: RESOLVED FIXED

[Tor Arne Vestbø]

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x00000000bbadbeef
0x0000000104ebeea1 in WebCore::TimerBase::stop (this=0x10ce237c8) at /Users/torarne/dev/webkit/wip/Source/WebCore/platform/Timer.cpp:219
219	    ASSERT(m_thread == currentThread());
(gdb) bt
#0  0x0000000104ebeea1 in WebCore::TimerBase::stop (this=0x10ce237c8) at /Users/torarne/dev/webkit/wip/Source/WebCore/platform/Timer.cpp:219
#1  0x0000000104ebede9 in WebCore::TimerBase::~TimerBase (this=0x10ce237c8) at /Users/torarne/dev/webkit/wip/Source/WebCore/platform/Timer.cpp:205
#2  0x000000010563ddd5 in WebCore::Timer<WebCore::GraphicsLayerTextureMapper>::~Timer (this=0x10ce237c8) at Timer.h:92
#3  0x000000010563cd15 in WebCore::Timer<WebCore::GraphicsLayerTextureMapper>::~Timer (this=0x10ce237c8) at Timer.h:92
#4  0x000000010563b97b in WebCore::GraphicsLayerTextureMapper::~GraphicsLayerTextureMapper (this=0x10ce23550) at /Users/torarne/dev/webkit/wip/Source/WebCore/platform/graphics/texmap/GraphicsLayerTextureMapper.cpp:62
#5  0x000000010563b935 in WebCore::GraphicsLayerTextureMapper::~GraphicsLayerTextureMapper (this=0x10ce23550) at /Users/torarne/dev/webkit/wip/Source/WebCore/platform/graphics/texmap/GraphicsLayerTextureMapper.cpp:60
#6  0x000000010563b8b8 in WebCore::GraphicsLayerTextureMapper::~GraphicsLayerTextureMapper (this=0x10ce23550) at /Users/torarne/dev/webkit/wip/Source/WebCore/platform/graphics/texmap/GraphicsLayerTextureMapper.cpp:60
#7  0x0000000103f8b4ce in WTF::deleteOwnedPtr<WebCore::GraphicsLayer> (ptr=0x10ce23550) at OwnPtrCommon.h:55
#8  0x0000000103f8b498 in WTF::OwnPtr<WebCore::GraphicsLayer>::~OwnPtr (this=0x10e8334c0) at OwnPtr.h:55
#9  0x0000000103f8a935 in WTF::OwnPtr<WebCore::GraphicsLayer>::~OwnPtr (this=0x10e8334c0) at OwnPtr.h:55
#10 0x00000001040d2448 in WebKit::WebLayerTreeRenderer::~WebLayerTreeRenderer (this=0x10e8333b0) at /Users/torarne/dev/webkit/wip/Source/WebKit2/UIProcess/WebLayerTreeRenderer.cpp:95
#11 0x00000001040d23d5 in WebKit::WebLayerTreeRenderer::~WebLayerTreeRenderer (this=0x10e8333b0) at /Users/torarne/dev/webkit/wip/Source/WebKit2/UIProcess/WebLayerTreeRenderer.cpp:94
#12 0x00000001040d2358 in WebKit::WebLayerTreeRenderer::~WebLayerTreeRenderer (this=0x10e8333b0) at /Users/torarne/dev/webkit/wip/Source/WebKit2/UIProcess/WebLayerTreeRenderer.cpp:94
#13 0x00000001040342f3 in WTF::ThreadSafeRefCounted<WebKit::WebLayerTreeRenderer>::deref (this=0x10e8333b8) at ThreadSafeRefCounted.h:137
#14 0x000000010406f7ac in WTF::RefAndDeref<WebKit::WebLayerTreeRenderer*, true>::deref (t=0x10e8333b0) at Functional.h:286
#15 0x0000000104070c46 in WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (WebKit::WebLayerTreeRenderer::*)()>, void ()(WebKit::WebLayerTreeRenderer*)>::~BoundFunctionImpl (this=0x10eb439a0) at Functional.h:367
#16 0x0000000104070ad5 in WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (WebKit::WebLayerTreeRenderer::*)()>, void ()(WebKit::WebLayerTreeRenderer*)>::~BoundFunctionImpl (this=0x10eb439a0) at Functional.h:366
#17 0x0000000104070af8 in WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (WebKit::WebLayerTreeRenderer::*)()>, void ()(WebKit::WebLayerTreeRenderer*)>::~BoundFunctionImpl (this=0x10eb439a0) at Functional.h:366
#18 0x0000000103fb63a3 in WTF::ThreadSafeRefCounted<WTF::FunctionImplBase>::deref (this=0x10eb439a8) at ThreadSafeRefCounted.h:137
#19 0x0000000103fb6c6b in WTF::derefIfNotNull<WTF::FunctionImplBase> (ptr=0x10eb439a0) at PassRefPtr.h:52
#20 0x0000000103fbc488 in WTF::RefPtr<WTF::FunctionImplBase>::~RefPtr (this=0x10eb1cfd0) at RefPtr.h:56
#21 0x0000000103fbc465 in WTF::RefPtr<WTF::FunctionImplBase>::~RefPtr (this=0x10eb1cfd0) at RefPtr.h:56
#22 0x0000000103fbc445 in WTF::FunctionBase::~FunctionBase (this=0x10eb1cfd0) at Functional.h:535
#23 0x0000000103fbc425 in WTF::Function<void ()()>::~Function (this=0x10eb1cfd0) at Functional.h:562
#24 0x0000000103faf8d5 in WTF::Function<void ()()>::~Function (this=0x10eb1cfd0) at Functional.h:562
#25 0x0000000105f166eb in callFunctionObject (context=0x10eb1cfd0) at /Users/torarne/dev/webkit/wip/Source/WTF/wtf/MainThread.cpp:226
#26 0x0000000105f15ed5 in WTF::dispatchFunctionsFromMainThread () at /Users/torarne/dev/webkit/wip/Source/WTF/wtf/MainThread.cpp:156
#27 0x0000000105f2328c in WTF::MainThreadInvoker::event (this=0x10e8278a0, e=0x10eb2db80) at /Users/torarne/dev/webkit/wip/Source/WTF/wtf/qt/MainThreadQt.cpp:60
#28 0x00000001016c36d8 in QCoreApplicationPrivate::notify_helper (this=0x10cc1dd10, receiver=0x10e8278a0, event=0x10eb2db80) at /Users/torarne/dev/qt/5.0/qtbase/src/corelib/kernel/qcoreapplication.cpp:810
#29 0x00000001016c362a in QCoreApplication::notify (this=0x7fff5fbfd300, receiver=0x10e8278a0, event=0x10eb2db80) at /Users/torarne/dev/qt/5.0/qtbase/src/corelib/kernel/qcoreapplication.cpp:756
#30 0x0000000101e435b8 in QGuiApplication::notify (this=0x7fff5fbfd300, object=0x10e8278a0, event=0x10eb2db80) at /Users/torarne/dev/qt/5.0/qtbase/src/gui/kernel/qguiapplication.cpp:901
#31 0x00000001016c34af in QCoreApplication::notifyInternal (this=0x7fff5fbfd300, receiver=0x10e8278a0, event=0x10eb2db80) at /Users/torarne/dev/qt/5.0/qtbase/src/corelib/kernel/qcoreapplication.cpp:694
#32 0x00000001016c7e04 in QCoreApplication::sendEvent (receiver=0x10e8278a0, event=0x10eb2db80) at qcoreapplication.h:210
#33 0x00000001016c4960 in QCoreApplicationPrivate::sendPostedEvents (receiver=0x0, event_type=0x0, data=0x10ce00a10) at /Users/torarne/dev/qt/5.0/qtbase/src/corelib/kernel/qcoreapplication.cpp:1293
#34 0x00000001016c3de8 in QCoreApplication::sendPostedEvents (receiver=0x0, event_type=0x0) at /Users/torarne/dev/qt/5.0/qtbase/src/corelib/kernel/qcoreapplication.cpp:1154
#35 0x0000000101e3092f in QWindowSystemInterface::sendWindowSystemEvents (eventDispatcher=0x10eb17310, flags={i = 0x0}) at /Users/torarne/dev/qt/5.0/qtbase/src/gui/kernel/qwindowsysteminterface_qpa.cpp:452
#36 0x000000010ee0faf2 in QCocoaEventDispatcherPrivate::processPostedEvents ()
#37 0x000000010ee10241 in QCocoaEventDispatcherPrivate::postedEventsSourcePerformCallback ()
#38 0x00007fff91ff96e1 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ ()
#39 0x00007fff91ff8f4d in __CFRunLoopDoSources0 ()
#40 0x00007fff9201fd39 in __CFRunLoopRun ()
#41 0x00007fff9201f676 in CFRunLoopRunSpecific ()
#42 0x00007fff8f68b31f in RunCurrentEventLoopInMode ()
#43 0x00007fff8f69251b in ReceiveNextEventCommon ()
#44 0x00007fff8f692456 in BlockUntilNextEventMatchingListInMode ()
#45 0x00007fff8d207f5d in _DPSNextEvent ()
#46 0x00007fff8d207861 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#47 0x00007fff8d20419d in -[NSApplication run] ()
#48 0x000000010ee0f20a in QCocoaEventDispatcher::processEvents ()
#49 0x00000001016beaec in QEventLoop::processEvents (this=0x7fff5fbfd090, flags={i = 0x24}) at /Users/torarne/dev/qt/5.0/qtbase/src/corelib/kernel/qeventloop.cpp:135
#50 0x00000001016bed08 in QEventLoop::exec (this=0x7fff5fbfd090, flags={i = 0x0}) at /Users/torarne/dev/qt/5.0/qtbase/src/corelib/kernel/qeventloop.cpp:211
#51 0x0000000100019196 in quick_test_main (argc=0x2, argv=0x7fff5fbfd320, name=0x10000aba0 "qmltests", createViewport=0, sourceDir=0x10000aba9 "/Users/torarne/dev/webkit/wip/Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView") at /Users/torarne/dev/qt/5.0/qtdeclarative/src/qmltest/quicktest.cpp:293
#52 0x0000000100008906 in main ()

[Viatcheslav Ostapenko]

Created attachment 137856 [details]
Patch

[Noam Rosenthal]

Comment on attachment 137856 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=137856&action=review

Are we sure that the web process is suspended between purgeGLResources and the next update?

> Source/WebKit2/UIProcess/WebLayerTreeRenderer.cpp:289
> +    if (!layer)

Comment

> Source/WebKit2/UIProcess/WebLayerTreeRenderer.cpp:303
> +    if (backingStore)

Add a comment about why this might be null.

> Source/WebKit2/UIProcess/WebLayerTreeRenderer.cpp:310
> +    if (backingStore)

Add a comment about why this might be null.

[Viatcheslav Ostapenko]

(In reply to comment #2)
> (From update of attachment 137856 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=137856&action=review
> 
> Are we sure that the web process is suspended between purgeGLResources and the next update?

The whole webprocess is not suspended, but rendering on drawing area should be. I don't know any scenario when scenegraph would delete paint node of visible item.

[Noam Rosenthal]

Comment on attachment 137856 [details]
Patch

r=me
Please add some comments in places where we return null, explaining that those updates come when the view is invisible.

[Noam Rosenthal]

Comment on attachment 137856 [details]
Patch

On second thought, this is bug-prone.
Let's make sure we don't do anything with the render-queue if the page is invisible.

[Viatcheslav Ostapenko]

Created attachment 137918 [details]
Remove null checks and make sure that queue is empty after paint node was deleted

[Noam Rosenthal]

Comment on attachment 137918 [details]
Remove null checks and make sure that queue is empty after paint node was deleted

View in context: https://bugs.webkit.org/attachment.cgi?id=137918&action=review

We should also ignore new incoming LayerTreeHostProxy messages until the next updatePaintNode.

> Source/WebKit2/UIProcess/WebLayerTreeRenderer.cpp:431
> +void WebLayerTreeRenderer::clearUpdateQueue()

Maybe clearRenderQueue, considering the line below?

[Viatcheslav Ostapenko]

Created attachment 137947 [details]
Ignore updates if paint node is deleted until recreated.

[Noam Rosenthal]

Comment on attachment 137947 [details]
Ignore updates if paint node is deleted until recreated.

View in context: https://bugs.webkit.org/attachment.cgi?id=137947&action=review

> Source/WebKit2/UIProcess/WebLayerTreeRenderer.cpp:441
> +void WebLayerTreeRenderer::clearRenderQueue()
> +{
> +    m_isActive = false;
> +    m_renderQueue.clear();
> +}
> +
> +void WebLayerTreeRenderer::activate()

These can be one function with a bool argument, that also call m_renderQueue.clear(). That way you don't have to call clearRenderQueue before activate().

[Noam Rosenthal]

Comment on attachment 137947 [details]
Ignore updates if paint node is deleted until recreated.

Please fix previous comments, otherwise r=me

[Viatcheslav Ostapenko]

Created attachment 138009 [details]
Updated patch by Noam comments.

[WebKit Review Bot]

Comment on attachment 138009 [details]
Updated patch by Noam comments.

Clearing flags on attachment: 138009

Committed r114705: <http://trac.webkit.org/changeset/114705>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.