Bug 222712 - REGRESSION (r272300): [iOS] ASSERTION FAILED: Unsafe to ref/deref from different threads under WebViewLayerFlushScheduler::layerFlushCallback
Summary: REGRESSION (r272300): [iOS] ASSERTION FAILED: Unsafe to ref/deref from differ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-03 23:30 PST by Ryan Haddad
Modified: 2021-03-16 11:39 PDT (History)
5 users (show)

See Also:


Attachments
patch (2.15 KB, patch)
2021-03-16 08:26 PDT, Antti Koivisto
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (2.15 KB, patch)
2021-03-16 08:59 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Haddad 2021-03-03 23:30:13 PST
The following test is consistently asserting on iOS debug bots:

    TestWebKitAPI.QuickLook.LegacyQuickLookContent
        2021-03-03 22:29:42.271 TestWebKitAPI[38997:42783719] nil host used in call to allowsSpecificHTTPSCertificateForHost
        2021-03-03 22:29:42.271 TestWebKitAPI[38997:42783719] nil host used in call to allowsAnyHTTPSCertificateForHost:
        ASSERTION FAILED: Unsafe to ref/deref from different threads
        m_isOwnedByMainThread == isMainThread()
        /Volumes/Data/worker/ios-simulator-14-debug/build/WebKitBuild/Debug-iphonesimulator/usr/local/include/wtf/RefCounted.h(114) : void WTF::RefCountedBase::applyRefDerefThreadingCheck() const
        1   0x109db4b09 WTFCrash
        2   0x13dee9321 WTF::RefCountedBase::applyRefDerefThreadingCheck() const
        3   0x13dee90fc WTF::RefCountedBase::derefBase() const
        4   0x13e0a0c7f WTF::RefCounted<LayerFlushController, std::__1::default_delete<LayerFlushController> >::deref() const
        5   0x13e0a5fe1 WTF::DefaultRefDerefTraits<LayerFlushController>::derefIfNotNull(LayerFlushController*)
        6   0x13e0ae819 WTF::RefPtr<LayerFlushController, WTF::RawPtrTraits<LayerFlushController>, WTF::DefaultRefDerefTraits<LayerFlushController> >::~RefPtr()
        7   0x13e0ae685 WTF::RefPtr<LayerFlushController, WTF::RawPtrTraits<LayerFlushController>, WTF::DefaultRefDerefTraits<LayerFlushController> >::~RefPtr()
        8   0x13e1639aa WebViewLayerFlushScheduler::layerFlushCallback()
        9   0x13e164b88 WebViewLayerFlushScheduler::WebViewLayerFlushScheduler(LayerFlushController*)::$_0::operator()() const
        10  0x13e164b3e WTF::Detail::CallableWrapper<WebViewLayerFlushScheduler::WebViewLayerFlushScheduler(LayerFlushController*)::$_0, void>::call()
        11  0x1212f44e2 WTF::Function<void ()>::operator()() const
        12  0x1251e6840 WebCore::RunLoopObserver::runLoopObserverFired()
        13  0x1251e67a0 WebCore::RunLoopObserver::runLoopObserverFired(__CFRunLoopObserver*, unsigned long, void*)
        14  0x13c6621e8 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__
        15  0x13c65ca67 __CFRunLoopDoObservers
        16  0x13c65c742 CFRunLoopRunSpecific
        17  0x1090147b9 -[NSRunLoop(NSRunLoop) runMode:beforeDate:]
        18  0x10470fe5e TestWebKitAPI::Util::run(bool*)
        19  0x1044b6dd6 QuickLook_LegacyQuickLookContent_Test::TestBody()
        20  0x1048afe54 void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*)
        21  0x10488cebb void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*)
        22  0x10488cdf6 testing::Test::Run()
        23  0x10488dc2a testing::TestInfo::Run()
        24  0x10488ea54 testing::TestCase::Run()
        25  0x1048998c8 testing::internal::UnitTestImpl::RunAllTests()
        26  0x1048b4524 bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*)
        27  0x1048993db bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*)
        28  0x1048992ad testing::UnitTest::Run()
        29  0x1046b9e51 RUN_ALL_TESTS()
        30  0x1046b9de2 TestWebKitAPI::TestsController::run(int, char**)
        31  0x10485fd55 main
        Child process terminated with signal 11: Segmentation fault


https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.QuickLook.LegacyQuickLookContent
Comment 1 Radar WebKit Bug Importer 2021-03-03 23:30:23 PST
<rdar://problem/75022845>
Comment 2 Ryan Haddad 2021-03-03 23:32:19 PST
The two changes between the last passing and first asserting runs are
https://trac.webkit.org/changeset/272300/webkit
https://trac.webkit.org/changeset/272299/webkit

I'm guessing it is related to https://trac.webkit.org/changeset/272300/webkit
Comment 3 Ryan Haddad 2021-03-03 23:34:29 PST
This is also seen with 

    TestWebKitAPI.CopyHTML.SanitizationPreservesCharacterSet
        2021-03-03 22:02:57.378 TestWebKitAPI[30721:41622219] PBItemCollectionServicer connection disconnected.
        2021-03-03 22:02:57.456 TestWebKitAPI[30721:41622219] PBItemCollectionServicer connection disconnected.
        ASSERTION FAILED: Unsafe to ref/deref from different threads
        m_isOwnedByMainThread == isMainThread()
        /Volumes/Data/worker/ios-simulator-14-debug/build/WebKitBuild/Debug-iphonesimulator/usr/local/include/wtf/RefCounted.h(114) : void WTF::RefCountedBase::applyRefDerefThreadingCheck() const

https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.CopyHTML.SanitizationPreservesCharacterSet
Comment 4 Simon Fraser (smfr) 2021-03-04 09:36:26 PST
My guess it that the real bug is that QuckLook is triggering off-main-thread work.
Comment 5 Antti Koivisto 2021-03-10 13:03:40 PST
(lldb) p isMainThread()
(bool) $5 = false
(lldb) bt 
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
  * frame #0: 0x0000000142553020 WebKitLegacy`LayerFlushController::ref(this=0x000000015b40f370) const at WebViewData.h:135:36
    frame #1: 0x0000000142552ffe WebKitLegacy`WTF::DefaultRefDerefTraits<LayerFlushController>::refIfNotNull(ptr=0x000000015b40f370) at RefPtr.h:36:18
    frame #2: 0x0000000142552fc4 WebKitLegacy`WTF::RefPtr<LayerFlushController, WTF::RawPtrTraits<LayerFlushController>, WTF::DefaultRefDerefTraits<LayerFlushController> >::RefPtr(this=0x00007ffee4bb8e30, ptr=0x000000015b40f370) at RefPtr.h:61:49
    frame #3: 0x0000000142550b6d WebKitLegacy`WTF::RefPtr<LayerFlushController, WTF::RawPtrTraits<LayerFlushController>, WTF::DefaultRefDerefTraits<LayerFlushController> >::RefPtr(this=0x00007ffee4bb8e30, ptr=0x000000015b40f370) at RefPtr.h:61:47
    frame #4: 0x0000000142550a8d WebKitLegacy`WebViewLayerFlushScheduler::layerFlushCallback(this=0x000000015b40f380) at WebViewData.mm:126:50
    frame #5: 0x0000000142551cf8 WebKitLegacy`WebViewLayerFlushScheduler::WebViewLayerFlushScheduler(this=0x00000001615e1068)::$_0::operator()() const at WebViewData.mm:102:15
    frame #6: 0x0000000142551cae WebKitLegacy`WTF::Detail::CallableWrapper<WebViewLayerFlushScheduler::WebViewLayerFlushScheduler(LayerFlushController*)::$_0, void>::call(this=0x00000001615e1060) at Function.h:52:39
    frame #7: 0x0000000126c7b322 WebCore`WTF::Function<void ()>::operator(this=0x000000015b405a40)() const at Function.h:83:35
    frame #8: 0x000000012a7e8420 WebCore`WebCore::RunLoopObserver::runLoopObserverFired(this=0x000000015b405a38) at RunLoopObserver.cpp:44:5
    frame #9: 0x000000012a7e8380 WebCore`WebCore::RunLoopObserver::runLoopObserverFired((null)=0x00007fda2f31c2d0, (null)=128, context=0x000000015b405a38) at RunLoopObserver.cpp:38:45
    frame #10: 0x0000000140346d69 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 23
    frame #11: 0x000000014034157a CoreFoundation`__CFRunLoopDoObservers + 541
    frame #12: 0x000000014034125b CoreFoundation`CFRunLoopRunSpecific + 691
    frame #13: 0x00000001517c755c UIFoundation`-[NSHTMLReader _loadUsingWebKit] + 1847
    frame #14: 0x00000001517c84a1 UIFoundation`-[NSHTMLReader attributedString] + 22
    frame #15: 0x0000000151748d63 UIFoundation`_NSReadAttributedStringFromURLOrData + 10026
    frame #16: 0x00000001517465c4 UIFoundation`-[NSAttributedString(NSAttributedStringUIFoundationAdditions) initWithData:options:documentAttributes:error:] + 144
    frame #17: 0x000000010b1f8c22 TestWebKitAPI`CopyHTML_SanitizationPreservesCharacterSet_Test::TestBody(this=0x00007fda2dd30a60) at CopyHTML.mm:160:41
    frame #18: 0x000000010ba3c594 TestWebKitAPI`void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(object=0x00007fda2dd30a60, method=21 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00, location="the test body")(), char const*) at gtest.cc:2443:10
    frame #19: 0x000000010ba1aecb TestWebKitAPI`void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(object=0x00007fda2dd30a60, method=21 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00, location="the test body")(), char const*) at gtest.cc:2479:14
    frame #20: 0x000000010ba1ae06 TestWebKitAPI`testing::Test::Run(this=0x00007fda2dd30a60) at gtest.cc:2517:5
    frame #21: 0x000000010ba1bc2a TestWebKitAPI`testing::TestInfo::Run(this=0x00007fda2f30e920) at gtest.cc:2693:11
    frame #22: 0x000000010ba1ca54 TestWebKitAPI`testing::TestCase::Run(this=0x00007fda2f30e2f0) at gtest.cc:2811:28
    frame #23: 0x000000010ba27798 TestWebKitAPI`testing::internal::UnitTestImpl::RunAllTests(this=0x00007fda2dd08150) at gtest.cc:5177:43
    frame #24: 0x000000010ba40c24 TestWebKitAPI`bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(object=0x00007fda2dd08150, method=10 75 a2 0b 01 00 00 00 00 00 00 00 00 00 00 00, location="auxiliary test code (environments or event listeners)")(), char const*) at gtest.cc:2443:10
    frame #25: 0x000000010ba272ab TestWebKitAPI`bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(object=0x00007fda2dd08150, method=10 75 a2 0b 01 00 00 00 00 00 00 00 00 00 00 00, location="auxiliary test code (environments or event listeners)")(), char const*) at gtest.cc:2479:14
    frame #26: 0x000000010ba2717d TestWebKitAPI`testing::UnitTest::Run(this=0x000000010bd6e138) at gtest.cc:4786:10
    frame #27: 0x000000010b852e41 TestWebKitAPI`RUN_ALL_TESTS() at gtest.h:2341:46
    frame #28: 0x000000010b852dd2 TestWebKitAPI`TestWebKitAPI::TestsController::run(this=0x000000010bd6de98, argc=1, argv=0x00007ffee4bba2e8) at TestsController.cpp:90:13
    frame #29: 0x000000010b9ee505 TestWebKitAPI`main(argc=2, argv=0x00007ffee4bba2e8) at mainIOS.mm:49:62
    frame #30: 0x0000000141655bbd libdyld.dylib`start + 1
    frame #31: 0x0000000141655bbd libdyld.dylib`start + 1
Comment 6 Antti Koivisto 2021-03-10 13:06:38 PST
We are in the main thread but isMainThread() is returning false. This then confuses the thread assertion in RefCountedBase where we do this:

    void applyRefDerefThreadingCheck() const
    {
#if ASSERT_ENABLED
        if (hasOneRef()) {
            // Likely an ownership transfer across threads that may be safe.
            m_isOwnedByMainThread = isMainThread();

The reason isMainThread() returns false is that the web thread lock is not being held in the runloop observer callback 

bool isMainThread()
{
    return (isWebThread() || pthread_main_np()) && webThreadIsUninitializedOrLockedOrDisabled();
}
Comment 7 Antti Koivisto 2021-03-16 07:38:18 PDT
The issue is that we are scheduling a layer flush from a layerFlushCallback and so scheduling a RunLoopObserver. 

This is fine in itself but becomes a problem when the main thread weblock auto-unlock observer runs before that observer. Then that layer flush runs without weblock held and causes corruption.

        1   0x13933141c WebViewLayerFlushScheduler::schedule()
        2   0x1393313ae LayerFlushController::scheduleLayerFlush()
        3   0x139262d59 -[WebView(WebViewInternal) _scheduleUpdateRendering]
        4   0x13928bce4 WebChromeClient::triggerRenderingUpdate()
        5   0x12064b52d WebCore::RenderingUpdateScheduler::triggerRenderingUpdate()
        6   0x12064b43e WebCore::RenderingUpdateScheduler::scheduleRenderingUpdate()
        7   0x120595a43 WebCore::Page::scheduleRenderingUpdateInternal()
        8   0x120595813 WebCore::Page::scheduleRenderingUpdate(WTF::OptionSet<WebCore::RenderingUpdateStep>)
        9   0x11f7567b1 WebCore::Document::scheduleRenderingUpdate(WTF::OptionSet<WebCore::RenderingUpdateStep>)
        10  0x121244a45 WebCore::Style::Scope::scheduleUpdate(WebCore::Style::Scope::UpdateType)
        11  0x1212452bc WebCore::Style::Scope::didChangeStyleSheetEnvironment()
        12  0x120544a54 WebCore::FrameViewLayoutContext::updateStyleForLayout()
        13  0x12052437a WebCore::FrameViewLayoutContext::layout()
        14  0x120537fe9 WebCore::FrameView::updateContentsSize()
        15  0x120541fc9 WebCore::FrameView::setCustomFixedPositionLayoutRect(WebCore::IntRect const&)
        16  0x139250486 -[WebView(WebPrivate) _synchronizeCustomFixedPositionLayoutRect]
        17  0x139244bd4 -[WebView(WebPrivate) _updateRendering]
        18  0x139262c51 LayerFlushController::flushLayers()
        19  0x139331963 WebViewLayerFlushScheduler::layerFlushCallback()
        20  0x139332ba8 WebViewLayerFlushScheduler::WebViewLayerFlushScheduler(LayerFlushController*)::$_0::operator()() const
        21  0x139332b5e WTF::Detail::CallableWrapper<WebViewLayerFlushScheduler::WebViewLayerFlushScheduler(LayerFlushController*)::$_0, void>::call()
        22  0x11cc8d2a2 WTF::Function<void ()>::operator()() const
        23  0x1207fa3d0 WebCore::RunLoopObserver::runLoopObserverFired()
        24  0x1207fa330 WebCore::RunLoopObserver::runLoopObserverFired(__CFRunLoopObserver*, unsigned long, void*)
        25  0x1370e0d69 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__
        26  0x1370db57a __CFRunLoopDoObservers
        27  0x1370dbb2d __CFRunLoopRun
Comment 8 Antti Koivisto 2021-03-16 08:26:11 PDT
Created attachment 423333 [details]
patch
Comment 9 Antti Koivisto 2021-03-16 08:59:22 PDT
Created attachment 423338 [details]
patch
Comment 10 Simon Fraser (smfr) 2021-03-16 10:25:49 PDT
Comment on attachment 423338 [details]
patch

r=me but please do some manual testing in UIWebView to ensure this doesn't cause any new delays or deadlocks.
Comment 11 EWS 2021-03-16 11:39:24 PDT
Committed r274500: <https://commits.webkit.org/r274500>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423338 [details].