Bug 208784

Summary: REGRESSION (r258064): API tests intermittently crashing under WebKit::registerDefaultsOverride
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: New BugsAssignee: Per Arne Vollan <pvollan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, darin, pvollan, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=208690
Attachments:
Description Flags
crash log
none
Patch
darin: review+
Patch none

Ryan Haddad
Reported 2020-03-08 09:42:56 PDT
Created attachment 392957 [details] crash log As seen here, API tests have been intermittently crashing on macOS Debug queues after r258064: https://results.webkit.org/suites?platform=mac&style=debug&suite=api-tests It seems to frequently be different tests, I don't see a theme. Here are the ones that crashed on a couple of different runs: TestWebKitAPI.SOAuthorizationRedirect.NSNotificationCenter TestWebKitAPI.WebKit.QuotaDelegateReload TestWebKitAPI.ServiceWorkers.ThrottleCrash TestWebKitAPI.ServiceWorkers.DifferentSessionsUseDifferentRegistrations TestWebKitAPI.DownloadProgress.PublishProgressOnPartialDownload TestWebKitAPI.WebKit.UploadDirectory TestWebKitAPI.WKAttachmentTests.AttachmentUpdatesWhenUndoingAndRedoing Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 ??? 000000000000000000 0 + 0 1 com.apple.WebKit 0x0000000111f41e5a void wtfCallIMP<void, NSDictionary<NSString*, objc_object*>*>(void (*)(), objc_object*, objc_selector*, NSDictionary<NSString*, objc_object*>*) + 42 (ObjCRuntimeExtras.h:40) 2 com.apple.WebKit 0x0000000111f41983 registerDefaultsOverride(objc_object*, objc_selector*, NSDictionary<NSString*, objc_object*>*) + 51 (PreferenceObserver.mm:40) 3 com.apple.AppKit 0x00007fff33327db6 ___NSWindowFetchCoreUIShadowMeasurements_block_invoke + 6619 4 libdispatch.dylib 0x00007fff6d64550e _dispatch_client_callout + 8 5 libdispatch.dylib 0x00007fff6d646686 _dispatch_once_callout + 20 6 com.apple.AppKit 0x00007fff3332625a _NSWindowConstructShadowParameters + 1987 7 com.apple.AppKit 0x00007fff33325a45 -[NSWindow shadowParameters] + 71 8 com.apple.AppKit 0x00007fff3332596a -[NSWindow _setShadowParameters] + 50 9 com.apple.AppKit 0x00007fff3331ef18 -[NSWindow _reallyDoOrderWindowAboveOrBelow:relativeTo:findKey:forCounter:force:isModal:] + 1576 10 com.apple.AppKit 0x00007fff3331e599 -[NSWindow _reallyDoOrderWindow:relativeTo:findKey:forCounter:force:isModal:] + 135 11 com.apple.AppKit 0x00007fff3331d505 -[NSWindow _doOrderWindow:relativeTo:findKey:forCounter:force:isModal:] + 283 12 com.apple.AppKit 0x00007fff3331d389 -[NSWindow orderWindow:relativeTo:] + 155 13 com.apple.AppKit 0x00007fff3330f8ed -[NSWindow makeKeyAndOrderFront:] + 60 14 TestWebKitAPI 0x000000010763ef54 -[TestWKWebView _setUpTestWindow:] + 388 15 TestWebKitAPI 0x000000010763ecd4 -[TestWKWebView initWithFrame:configuration:addToWindow:] + 292 (TestWKWebView.mm:443) 16 TestWebKitAPI 0x000000010763eb9d -[TestWKWebView initWithFrame:configuration:] + 125 (TestWKWebView.mm:416) 17 TestWebKitAPI 0x000000010763eb14 -[TestWKWebView initWithFrame:] + 148 (TestWKWebView.mm:411) 18 TestWebKitAPI 0x000000010714e38a WKWebView_FindAPIBackwardsNoWrap_Test::TestBody() + 154 (FindInPageAPI.mm:175) 19 TestWebKitAPI 0x000000010785e57e void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) + 126 (gtest.cc:2443) 20 TestWebKitAPI 0x000000010782c5cb void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) + 123 (gtest.cc:2479) 21 TestWebKitAPI 0x000000010782c4f6 testing::Test::Run() + 198 (gtest.cc:2524)
Attachments
crash log (72.16 KB, text/plain)
2020-03-08 09:42 PDT, Ryan Haddad 		no flags
Details
Patch (3.52 KB, patch)
2020-03-08 12:40 PDT, Per Arne Vollan 		darin: review+
DetailsFormatted DiffDiff
Patch (3.84 KB, patch)
2020-03-08 15:28 PDT, Per Arne Vollan 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2020-03-08 09:43:27 PDT
<rdar://problem/60201631>
Per Arne Vollan
Comment 2 2020-03-08 12:31:52 PDT
Looking into this now.
Per Arne Vollan
Comment 3 2020-03-08 12:40:35 PDT
Created attachment 392976 [details] Patch
Darin Adler
Comment 4 2020-03-08 13:01:26 PDT
Comment on attachment 392976 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=392976&action=review > Source/WebKit/UIProcess/Cocoa/PreferenceObserver.mm:107 > registerDefaultsOriginal = method_setImplementation(registerDefaultsMethod, (IMP)registerDefaultsOverride); Is this early enough? Seems like there is a race here if registerDefaults is being called on another thread. Doing it on the main thread is good, but also it needs to be really early, before other threads exist.
Per Arne Vollan
Comment 5 2020-03-08 15:28:51 PDT
Created attachment 392993 [details] Patch
Per Arne Vollan
Comment 6 2020-03-08 15:33:35 PDT
(In reply to Darin Adler from comment #4) > Comment on attachment 392976 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=392976&action=review > > > Source/WebKit/UIProcess/Cocoa/PreferenceObserver.mm:107 > > registerDefaultsOriginal = method_setImplementation(registerDefaultsMethod, (IMP)registerDefaultsOverride); > > Is this early enough? Seems like there is a race here if registerDefaults is > being called on another thread. Doing it on the main thread is good, but > also it needs to be really early, before other threads exist. Yes, this should be early enough. As long as we do it before we start observing preference changes, we should be fine! Thanks for reviewing!
Per Arne Vollan
Comment 7 2020-03-08 16:24:13 PDT
Committed r258119: <https://trac.webkit.org/changeset/258119/webkit>.
Note You need to log in before you can comment on or make changes to this bug.