WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(27.19 KB, patch)
2019-10-15 12:36 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(27.42 KB, patch)
2019-10-15 14:54 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2019-10-15 12:31:47 PDT
Created
attachment 381011
[details]
Patch
Chris Dumez
Comment 2
2019-10-15 12:36:12 PDT
Created
attachment 381012
[details]
Patch
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
Created
attachment 381026
[details]
Patch
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
<
rdar://problem/56313281
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug