ThreadSafeRefCounted<DestructionThread::Main> is not safe to use in the UIProcess because its implementation relies on isMainThread() / callOnMainThread(). Those get confused about which thread is the main thread when an application uses both WK1 and WK2.
<rdar://problem/52289717>
We can add an assertion to catch this?
Created attachment 373360 [details] Patch
(In reply to Ryosuke Niwa from comment #2) > We can add an assertion to catch this? I think it would be great yes, but I am not sure how to write it. I know similar issues came up in the past, did we come up with a solution to catch this?
(In reply to Chris Dumez from comment #4) > (In reply to Ryosuke Niwa from comment #2) > > We can add an assertion to catch this? > > I think it would be great yes, but I am not sure how to write it. I know > similar issues came up in the past, did we come up with a solution to catch > this? There is isInWebProcess() as a runtime check which is a bit cheesy. Ideally, it's something we initialize via XPC bootstrapping instead.
Comment on attachment 373360 [details] Patch How about WebResourceLoadStatisticsStore?
Also Storage in NetworkCacheStorage.
Also, RegistrationDatabase and ServiceWorkerFetch::Client in WebCore uses it. Doesn't UI process use any of them?
(In reply to Ryosuke Niwa from comment #6) > Comment on attachment 373360 [details] > Patch > > How about WebResourceLoadStatisticsStore? Looks like it is in the NetworkProcess noawadays?
(In reply to Chris Dumez from comment #4) > (In reply to Ryosuke Niwa from comment #2) > > We can add an assertion to catch this? > > I think it would be great yes, but I am not sure how to write it. I know > similar issues came up in the past, did we come up with a solution to catch > this? See https://bugs.webkit.org/show_bug.cgi?id=185330
(In reply to Ryosuke Niwa from comment #7) > Also Storage in NetworkCacheStorage. Again, this is in the NetworkProcess so not an issue.
(In reply to Ryosuke Niwa from comment #8) > Also, RegistrationDatabase and ServiceWorkerFetch::Client in WebCore uses > it. Doesn't UI process use any of them? RegistrationDatabase is used only in the NetworkProcess so not a problem. ServiceWorkerFetch::Client is used in the Service Worker process I believe (So a WebProcess), thus not an issue either.
(In reply to Chris Dumez from comment #12) > (In reply to Ryosuke Niwa from comment #8) > > Also, RegistrationDatabase and ServiceWorkerFetch::Client in WebCore uses > > it. Doesn't UI process use any of them? > > RegistrationDatabase is used only in the NetworkProcess so not a problem. > ServiceWorkerFetch::Client is used in the Service Worker process I believe > (So a WebProcess), thus not an issue either. To be clear, classes that are used in the Network or WebContent processes, and only by WebKit2 code could use either DestructionThread::Main or DestructionThreadMainRunLoop. They would both be safe AFAIK. The only problematic ones are the ones used in the UIProcess, and I fixed the 2 classes I that were obviously used in the UIProcess.
Comment on attachment 373360 [details] Patch Clearing flags on attachment: 373360 Committed r247078: <https://trac.webkit.org/changeset/247078>
All reviewed patches have been landed. Closing bug.
*** Bug 199657 has been marked as a duplicate of this bug. ***