Bug 227930 - REGRESSION (r279119?): [iOS] ASSERTION FAILED: !m_impl || !m_shouldEnableAssertions || m_impl->wasConstructedOnMainThread() == isMainThread() seen with 3 WebKitLegacy media API tests
Summary: REGRESSION (r279119?): [iOS] ASSERTION FAILED: !m_impl || !m_shouldEnableAsse...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-13 16:55 PDT by ayumi_kojima
Modified: 2021-07-15 09:16 PDT (History)
11 users (show)

See Also:


Attachments
crash log (185.04 KB, text/plain)
2021-07-13 17:18 PDT, Ryan Haddad
no flags Details
Patch (2.03 KB, patch)
2021-07-14 08:15 PDT, Jer Noble
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (2.04 KB, patch)
2021-07-14 08:33 PDT, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ayumi_kojima 2021-07-13 16:55:45 PDT
TestWebKitAPI.WebKitLegacy.AudioSessionCategoryIOS
TestWebKitAPI.WebKitLegacy.ScrollingDoesNotPauseMedia
TestWebKitAPI.WebKitLegacy.PreemptVideoFullscreen

History: https://results.webkit.org/?suite=api-tests&suite=api-tests&suite=api-tests&test=TestWebKitAPI.WebKitLegacy.AudioSessionCategoryIOS&test=TestWebKitAPI.WebKitLegacy.PreemptVideoFullscreen&test=TestWebKitAPI.WebKitLegacy.ScrollingDoesNotPauseMedia

Are crashing on iOS 14 E Simulator Debug on iPhone 8 & iPad Pro and timing out on iOS 14 E Simulator Release on iPhone 8.

===================================

TestWebKitAPI.WebKitLegacy.AudioSessionCategoryIOS
        2021-07-13 15:42:03.031 TestWebKitAPI[99626:160412768] nil host used in call to allowsSpecificHTTPSCertificateForHost
        2021-07-13 15:42:03.031 TestWebKitAPI[99626:160412768] nil host used in call to allowsAnyHTTPSCertificateForHost:
        ASSERTION FAILED: !m_impl || !m_shouldEnableAssertions || m_impl->wasConstructedOnMainThread() == isMainThread()
        /Volumes/Data/worker/ios-simulator-14-debug/build/WebKitBuild/Debug-iphonesimulator/usr/local/include/wtf/WeakPtr.h(118) : T *WTF::WeakPtr<WebCore::MediaPlayerPrivateAVFoundationObjC, WTF::EmptyCounter>::operator->() const [T = WebCore::MediaPlayerPrivateAVFoundationObjC, Counter = WTF::EmptyCounter]
        1   0x119753db9 WTFCrash

===================================

https://build.webkit.org/#/builders/59/builds/1987/steps/12/logs/stdio
Comment 1 Radar WebKit Bug Importer 2021-07-13 16:56:31 PDT
<rdar://problem/80545962>
Comment 2 ayumi_kojima 2021-07-13 16:58:44 PDT
It appears it has started in https://trac.webkit.org/changeset/279119/webkit
Comment 3 Jer Noble 2021-07-13 17:16:10 PDT
Fuller crashlog:

```
        /Volumes/Data/worker/ios-simulator-14-debug/build/WebKitBuild/Debug-iphonesimulator/usr/local/include/wtf/WeakPtr.h(118) : T *WTF::WeakPtr<WebCore::MediaPlayerPrivateAVFoundationObjC, WTF::EmptyCounter>::operator->() const [T = WebCore::MediaPlayerPrivateAVFoundationObjC, Counter = WTF::EmptyCounter]
        1   0x105f7fdb9 WTFCrash
        2   0x1211aeb1b WTFCrashWithInfo(int, char const*, char const*, int)
        3   0x121297067 WTF::WeakPtr<WebCore::MediaPlayerPrivateAVFoundationObjC, WTF::EmptyCounter>::operator->() const
        4   0x1215c60ba WebCore::MediaPlayerPrivateAVFoundationObjC::createAVPlayer()::$_4::operator()(CMTime) const
        5   0x1215c6079 invocation function for block in WebCore::MediaPlayerPrivateAVFoundationObjC::createAVPlayer()
        6   0x160c5ce8d -[AVPeriodicTimebaseObserver _effectiveRateChanged]
```

This doesn't seem related to the change in question. It seems much more likely that the cause was: https://trac.webkit.org/changeset/279146/webkit

Basically, we dispatch to the main thread, not the UI thread. We should probably re-dispatch to the UI thread if that's needed.
Comment 4 Ryan Haddad 2021-07-13 17:18:31 PDT
Created attachment 433458 [details]
crash log
Comment 5 Alexey Proskuryakov 2021-07-13 22:57:53 PDT
These tests fail consistently, and the bots are quite certain that the tests passed with r279117, and failed starting r279120 (no historical data in between).
Comment 6 Jer Noble 2021-07-14 07:58:40 PDT
(In reply to Alexey Proskuryakov from comment #5)
> These tests fail consistently, and the bots are quite certain that the tests
> passed with r279117, and failed starting r279120 (no historical data in
> between).

Be that as it may, the crash in question is in a weak pointer dereference that did not exist before r279146.
Comment 7 Jer Noble 2021-07-14 08:05:59 PDT
(In reply to Jer Noble from comment #6)
> (In reply to Alexey Proskuryakov from comment #5)
> > These tests fail consistently, and the bots are quite certain that the tests
> > passed with r279117, and failed starting r279120 (no historical data in
> > between).
> 
> Be that as it may, the crash in question is in a weak pointer dereference
> that did not exist before r279146.

Looks like crashes prior to r279146 were fixed by bug #227227 / r279200 (239087@main).
Comment 8 Jer Noble 2021-07-14 08:15:28 PDT
Created attachment 433501 [details]
Patch
Comment 9 Jer Noble 2021-07-14 08:33:35 PDT
Created attachment 433502 [details]
Patch
Comment 10 Alexey Proskuryakov 2021-07-14 12:15:33 PDT
These are excellent tests to have caught two regressions in one day!
Comment 11 EWS 2021-07-15 09:15:58 PDT
Committed r279949 (239692@main): <https://commits.webkit.org/239692@main>

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