Bug 197666 - Regression: Crash at WebKit: PAL::HysteresisActivity::start
Summary: Regression: Crash at WebKit: PAL::HysteresisActivity::start
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2019-05-07 10:49 PDT by Chris Dumez
Modified: 2019-05-08 14:18 PDT (History)
8 users (show)

See Also:


Attachments
Patch (12.70 KB, patch)
2019-05-07 11:00 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (12.70 KB, patch)
2019-05-07 11:43 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (12.73 KB, patch)
2019-05-07 13:33 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews214 for win-future (13.74 MB, application/zip)
2019-05-07 17:51 PDT, Build Bot
no flags Details
Patch (10.99 KB, patch)
2019-05-07 18:36 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (10.99 KB, patch)
2019-05-07 18:41 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (10.97 KB, patch)
2019-05-08 13:10 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2019-05-07 10:49:58 PDT
Regression: Crash at WebKit: PAL::HysteresisActivity::start:
Thread 0 name:  Dispatch queue: com.apple.main-thread
Thread 0 Crashed ↩:
0   WebKit                        	0x00000001c83f5530 PAL::HysteresisActivity::start() + 76 (Function.h:57)
1   WebKit                        	0x00000001c83f5514 PAL::HysteresisActivity::start() + 48 (HysteresisActivity.h:56)
2   JavaScriptCore                	0x00000001d33cc3e0 WTF::dispatchFunctionsFromMainThread() + 264 (Function.h:57)
3   libobjc.A.dylib               	0x00000001cf96d490 -[NSObject performSelector:withObject:] + 68 (NSObject.mm:2185)
4   Foundation                    	0x00000001cf06e834 __NSThreadPerformPerform + 236 (NSThread.m:807)
5   CoreFoundation                	0x00000001cf3cdea8 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 28 (CFRunLoop.c:1904)
6   CoreFoundation                	0x00000001cf3cde30 __CFRunLoopDoSource0 + 92 (CFRunLoop.c:1938)
7   CoreFoundation                	0x00000001cf3cd590 __CFRunLoopDoSources0 + 184 (CFRunLoop.c:1974)
8   CoreFoundation                	0x00000001cf3c8484 __CFRunLoopRun + 1068 (CFRunLoop.c:2864)
9   CoreFoundation                	0x00000001cf3c7d30 CFRunLoopRunSpecific + 480 (CFRunLoop.c:3174)
10  Foundation                    	0x00000001cef36e98 -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 232 (NSRunLoop.m:374)
11  Foundation                    	0x00000001cef71a50 -[NSRunLoop(NSRunLoop) run] + 92 (NSRunLoop.m:399)
12  libxpc.dylib                  	0x00000001cf9c632c _xpc_objc_main.cold.4 + 44
13  libxpc.dylib                  	0x00000001cf9b1a98 _xpc_objc_main + 304 (main.m:174)
14  libxpc.dylib                  	0x00000001cf9b4254 xpc_main + 152 (init.c:1522)
15  WebKit                        	0x00000001c8543bf4 WebKit::XPCServiceMain(int, char const**) + 376 (XPCServiceMain.mm:147)
16  libdyld.dylib                 	0x00000001cf931500 start + 4
Comment 1 Chris Dumez 2019-05-07 10:50:14 PDT
<rdar://problem/50037153>
Comment 2 Chris Dumez 2019-05-07 11:00:40 PDT
Created attachment 369302 [details]
Patch
Comment 3 Chris Dumez 2019-05-07 11:43:41 PDT
Created attachment 369307 [details]
Patch
Comment 4 Chris Dumez 2019-05-07 13:33:46 PDT
Created attachment 369323 [details]
Patch
Comment 5 Anders Carlsson 2019-05-07 14:38:04 PDT
Is it really OK to rely on pointer equality like this? What if the client gets deallocated and a new client with the same address gets deallocated?
Comment 6 Chris Dumez 2019-05-07 14:40:33 PDT
(In reply to Anders Carlsson from comment #5)
> Is it really OK to rely on pointer equality like this? What if the client
> gets deallocated and a new client with the same address gets deallocated?

It seems possible in theory. I'll find a more robust approach.
Comment 7 Build Bot 2019-05-07 17:51:33 PDT
Comment on attachment 369323 [details]
Patch

Attachment 369323 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12128243

New failing tests:
legacy-animation-engine/compositing/reflections/load-video-in-reflection.html
Comment 8 Build Bot 2019-05-07 17:51:35 PDT
Created attachment 369343 [details]
Archive of layout-test-results from ews214 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews214  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment 9 Chris Dumez 2019-05-07 18:36:05 PDT
Created attachment 369348 [details]
Patch
Comment 10 Chris Dumez 2019-05-07 18:41:53 PDT
Created attachment 369349 [details]
Patch
Comment 11 Geoffrey Garen 2019-05-08 10:52:44 PDT
Comment on attachment 369349 [details]
Patch

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

r=me

> Source/WebKit/ChangeLog:18
> +        1. The constructor takes in a WTF::Function instead of a NetworkProcess / WebProcess crash. This is provides

crash => reference

> Source/WebKit/Shared/WebSQLiteDatabaseTracker.cpp:52
> +        notifyClient(false);

I'd say

    bool isHoldingLockedFiles = false;
    notifyClient(isHoldingLockedFiles);

to avoid a cryptic boolean argument.

(I prefer this style to adding a comment about the meaning of the boolean argument. Code speaks. Comments mumble.)
Comment 12 Chris Dumez 2019-05-08 13:10:07 PDT
Created attachment 369411 [details]
Patch
Comment 13 WebKit Commit Bot 2019-05-08 14:18:55 PDT
Comment on attachment 369411 [details]
Patch

Clearing flags on attachment: 369411

Committed r245069: <https://trac.webkit.org/changeset/245069>
Comment 14 WebKit Commit Bot 2019-05-08 14:18:57 PDT
All reviewed patches have been landed.  Closing bug.