RESOLVED FIXED 203001
[macOS] Simplify main thread initialization
https://bugs.webkit.org/show_bug.cgi?id=203001
Summary [macOS] Simplify main thread initialization
Chris Dumez
Reported 2019-10-15 12:18:08 PDT
Simplify main thread initialization on macOS by always using pthread main as main thread.
Attachments
Patch (27.85 KB, patch)
2019-10-15 12:31 PDT, Chris Dumez
no flags
Patch (27.19 KB, patch)
2019-10-15 12:36 PDT, Chris Dumez
no flags
Patch (27.42 KB, patch)
2019-10-15 14:54 PDT, Chris Dumez
no flags
Chris Dumez
Comment 1 2019-10-15 12:31:47 PDT
Chris Dumez
Comment 2 2019-10-15 12:36:12 PDT
Geoffrey Garen
Comment 3 2019-10-15 13:56:05 PDT
Comment on attachment 381012 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381012&action=review r=me > Source/WTF/ChangeLog:10 > + Probably worth mentioning that this change also enables the new white box ASSERT that we don't initialize main thread to some other thread. > Source/WebKitLegacy/mac/Misc/WebCache.mm:-68 > JSC::initializeThreading(); > - WTF::initializeMainThreadToProcessMainThread(); In cases like this, is it OK to call JSC::initializeThreading() and not WTF::initializeMainThreadToProcessMainThread
Geoffrey Garen
Comment 4 2019-10-15 14:07:43 PDT
> > Source/WebKitLegacy/mac/Misc/WebCache.mm:-68 > > JSC::initializeThreading(); > > - WTF::initializeMainThreadToProcessMainThread(); > > In cases like this, is it OK to call JSC::initializeThreading() and not > WTF::initializeMainThreadToProcessMainThread I verified that JSC::initializeThreading(); calls WTF::initializeThreading(), so it's OK.
Chris Dumez
Comment 5 2019-10-15 14:51:32 PDT
Comment on attachment 381012 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381012&action=review >> Source/WTF/ChangeLog:10 >> + > > Probably worth mentioning that this change also enables the new white box ASSERT that we don't initialize main thread to some other thread. Will add. >> Source/WebKitLegacy/mac/Misc/WebCache.mm:-68 >> - WTF::initializeMainThreadToProcessMainThread(); > > In cases like this, is it OK to call JSC::initializeThreading() and not WTF::initializeMainThreadToProcessMainThread I dropped it because RunLoop::initializeMainRunLoop() below calls WTF::initializeMainThread(); With my change, WTF::initializeMainThread() became identical to WTF::initializeMainThreadToProcessMainThread() so I dropped the latter.
Chris Dumez
Comment 6 2019-10-15 14:54:55 PDT
WebKit Commit Bot
Comment 7 2019-10-15 15:44:03 PDT
Comment on attachment 381026 [details] Patch Clearing flags on attachment: 381026 Committed r251164: <https://trac.webkit.org/changeset/251164>
WebKit Commit Bot
Comment 8 2019-10-15 15:44:05 PDT
All reviewed patches have been landed. Closing bug.
Ryosuke Niwa
Comment 9 2019-10-15 16:05:33 PDT
This patch broke builds: ld: warning: directory not found for option '-F/Volumes/Data/slave/highsierra-debug/build/Tools/TestWebKitAPI/../../WebKitLibraries/WebKitPrivateFrameworkStubs/Mac/101300' Undefined symbols for architecture x86_64: "WTF::isMainThreadIfInitialized()", referenced from: WTF::RefCountedBase::applyRefDerefThreadingCheck() const in libTestWebKitAPI.a(EnvironmentUtilitiesTest.o) ld: symbol(s) not found for architecture x86_64 clang: error: linker command failed with exit code 1 (use -v to see invocation) Not sure why EWS didn't catch it.
Ryosuke Niwa
Comment 10 2019-10-15 16:07:44 PDT
Hm... I wonder if we just need a clean build?
Chris Dumez
Comment 11 2019-10-15 16:16:52 PDT
(In reply to Ryosuke Niwa from comment #10) > Hm... I wonder if we just need a clean build? Sounds likely.
Ryosuke Niwa
Comment 12 2019-10-15 16:34:51 PDT
(In reply to Chris Dumez from comment #11) > (In reply to Ryosuke Niwa from comment #10) > > Hm... I wonder if we just need a clean build? > > Sounds likely. Indeed, clean build fixed the issue for me.
Radar WebKit Bug Importer
Comment 13 2019-10-15 16:35:04 PDT
Note You need to log in before you can comment on or make changes to this bug.