Bug 213067

Summary: Occasional crashes when running nested runloops while using UIWebView
Product: WebKit Reporter: Tim Horton <thorton>
Component: New BugsAssignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, cdumez, darin, ddkilzer, ggaren, krollin, mitz, nham, simon.fraser, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 213072    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

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].