Bug 213067 - Occasional crashes when running nested runloops while using UIWebView
Summary: Occasional crashes when running nested runloops while using UIWebView
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on: 213072
Blocks:
  Show dependency treegraph
 
Reported: 2020-06-10 21:52 PDT by Tim Horton
Modified: 2020-08-03 15:53 PDT (History)
11 users (show)

See Also:


Attachments
Patch (10.50 KB, patch)
2020-06-10 21:56 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (11.37 KB, patch)
2020-06-10 22:49 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (22.05 KB, patch)
2020-06-15 18:44 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (14.06 KB, patch)
2020-08-03 14:44 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (14.05 KB, patch)
2020-08-03 14:52 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (14.05 KB, patch)
2020-08-03 14:54 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (14.02 KB, patch)
2020-08-03 15:05 PDT, Tim Horton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2020-06-10 21:52:28 PDT
Occasional crashes when running nested runloops while using UIWebView
Comment 1 Tim Horton 2020-06-10 21:56:20 PDT
Created attachment 401618 [details]
Patch
Comment 2 Tim Horton 2020-06-10 21:57:32 PDT
I am not a CFRunLoop expert by any stretch of the imagination, so I hope someone can tell me if I'm doing something horribly wrong, or if there's a good reason not to do it this way.
Comment 3 Radar WebKit Bug Importer 2020-06-10 21:58:30 PDT
<rdar://problem/64239727>
Comment 4 Geoffrey Garen 2020-06-10 22:21:51 PDT
Comment on attachment 401618 [details]
Patch

r=me
Comment 5 Geoffrey Garen 2020-06-10 22:22:21 PDT
I’ll CC Ben in case he can tell us that we did something wrong.
Comment 6 Tim Horton 2020-06-10 22:25:20 PDT
Ugh, I think the bots will need a clean build (or I will need to fix the build system). I had the same failure locally 'till I did.
Comment 7 Tim Horton 2020-06-10 22:26:51 PDT
Or maybe there's some file I can touch
Comment 8 Tim Horton 2020-06-10 22:49:04 PDT
Created attachment 401621 [details]
Patch
Comment 9 Simon Fraser (smfr) 2020-06-11 19:39:50 PDT
Comment on attachment 401618 [details]
Patch

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

> Source/WebCore/platform/ios/wak/WebCoreThread.mm:667
> +    RetainPtr<CFRunLoopObserverRef> mainRunLoopEntryObserver = adoptCF(CFRunLoopObserverCreate(nullptr, kCFRunLoopEntry, YES, 0, MainRunLoopEntry, nullptr));
> +    RetainPtr<CFRunLoopObserverRef> mainRunLoopExitObserver = adoptCF(CFRunLoopObserverCreate(nullptr, kCFRunLoopExit, YES, 3000002, MainRunLoopExit, nullptr));

auto?. What does 3000002 come from?
Comment 10 Keith Rollin 2020-06-12 00:22:08 PDT
With Bug 213072 landed, I'm retrying the previously-failed builds.
Comment 11 Darin Adler 2020-06-12 09:38:15 PDT
Comment on attachment 401618 [details]
Patch

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

>> Source/WebCore/platform/ios/wak/WebCoreThread.mm:667
>> +    RetainPtr<CFRunLoopObserverRef> mainRunLoopExitObserver = adoptCF(CFRunLoopObserverCreate(nullptr, kCFRunLoopExit, YES, 3000002, MainRunLoopExit, nullptr));
> 
> auto?. What does 3000002 come from?

I’d also like to learn about 3000002.
Comment 12 Tim Horton 2020-06-12 10:00:52 PDT
Comment on attachment 401618 [details]
Patch

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

>>> Source/WebCore/platform/ios/wak/WebCoreThread.mm:667
>>> +    RetainPtr<CFRunLoopObserverRef> mainRunLoopExitObserver = adoptCF(CFRunLoopObserverCreate(nullptr, kCFRunLoopExit, YES, 3000002, MainRunLoopExit, nullptr));
>> 
>> auto?. What does 3000002 come from?
> 
> I’d also like to learn about 3000002.

It's 3000001+1. 3000001 is 3000000+1.

Perhaps we should give these some names :)
Comment 13 Simon Fraser (smfr) 2020-06-12 11:14:24 PDT
(In reply to Tim Horton from comment #12)
> Comment on attachment 401618 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=401618&action=review
> 
> >>> Source/WebCore/platform/ios/wak/WebCoreThread.mm:667
> >>> +    RetainPtr<CFRunLoopObserverRef> mainRunLoopExitObserver = adoptCF(CFRunLoopObserverCreate(nullptr, kCFRunLoopExit, YES, 3000002, MainRunLoopExit, nullptr));
> >> 
> >> auto?. What does 3000002 come from?
> > 
> > I’d also like to learn about 3000002.
> 
> It's 3000001+1. 3000001 is 3000000+1.
> 
> Perhaps we should give these some names :)

You mean like WellKnownRunLoopOrders
Comment 14 Tim Horton 2020-06-12 11:22:53 PDT
Perfect! Also that suggests that 3000002 is actually 2000000+1000000+1 :)
Comment 15 Tim Horton 2020-06-12 11:23:05 PDT
(In reply to Tim Horton from comment #14)
> Perfect! Also that suggests that 3000002 is actually 2000000+1000000+1 :)

Err +2 obv
Comment 16 Tim Horton 2020-06-15 18:44:09 PDT
Created attachment 401965 [details]
Patch
Comment 17 Geoffrey Garen 2020-06-15 18:55:48 PDT
Comment on attachment 401965 [details]
Patch

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

r=me

> Source/WebCore/platform/ios/wak/WebCoreThread.mm:667
> +    RetainPtr<CFRunLoopObserverRef> mainRunLoopExitObserver = adoptCF(CFRunLoopObserverCreate(nullptr, kCFRunLoopExit, YES, 3000002, MainRunLoopExit, nullptr));

Did we promise we’d give this a name?
Comment 18 Tim Horton 2020-08-03 14:44:01 PDT
Created attachment 405865 [details]
Patch
Comment 19 Tim Horton 2020-08-03 14:52:28 PDT
Created attachment 405867 [details]
Patch
Comment 20 Tim Horton 2020-08-03 14:54:55 PDT
Created attachment 405869 [details]
Patch
Comment 21 Darin Adler 2020-08-03 14:58:08 PDT
Comment on attachment 405869 [details]
Patch

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

> Tools/TestWebKitAPI/Tests/WebKitLegacy/ios/WebThreadLock.mm:44
> +    RetainPtr<CFRunLoopObserverRef> observer = adoptCF(CFRunLoopObserverCreateWithHandler(NULL, kCFRunLoopBeforeWaiting, NO, 2, ^(CFRunLoopObserverRef observer, CFRunLoopActivity activity) {

auto
Comment 22 Tim Horton 2020-08-03 15:05:17 PDT
Created attachment 405871 [details]
Patch
Comment 23 EWS 2020-08-03 15:53:10 PDT
Committed r265229: <https://trac.webkit.org/changeset/265229>

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