| Summary: | Implement SharedWorker | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alex Christensen <achristensen> | ||||||||
| Component: | New Bugs | Assignee: | Chris Dumez <cdumez> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | annulen, bfulgham, carlo.patti, cdumez, dst.germain48, eric.carlson, esprehn+autocc, ews-watchlist, ggaren, glenn, gyuyoung.kim, ian, jdscheff, jer.noble, kangil.han, koddsson, kondapallykalyan, matt, mihaip, mkwst, philipj, ryuan.choi, sergio, tobiasuhlig78, tony, tregoning, vepomoc, webkit-bug-importer, Zhenya.kovalev | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=149850 https://bugs.webkit.org/show_bug.cgi?id=233848 |
||||||||||
| Bug Depends on: | 233769, 233772, 233793, 233848, 233895, 235958, 236271, 236302, 236317, 236319, 236329, 236334, 236340, 236396, 236411, 236419 | ||||||||||
| Bug Blocks: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Alex Christensen
2021-09-16 18:12:21 PDT
Created attachment 438425 [details]
Patch
Thread 15 Crashed:: WebCore: Worker 0 libsystem_kernel.dylib 0x00007fff692bb33a __pthread_kill + 10 1 libsystem_pthread.dylib 0x00007fff6937be60 pthread_kill + 430 2 libsystem_c.dylib 0x00007fff69242808 abort + 120 3 libc++abi.dylib 0x00007fff664ab458 abort_message + 231 4 libc++abi.dylib 0x00007fff664aae92 __cxa_pure_virtual + 18 5 com.apple.WebCore 0x0000000149d71ce9 WebCore::WorkerOrWorkletThread::workerOrWorkletThread() + 57 6 com.apple.JavaScriptCore 0x000000014d624bbc WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) + 124 7 com.apple.JavaScriptCore 0x000000014d627449 WTF::wtfThreadEntryPoint(void*) + 9 8 libsystem_pthread.dylib 0x00007fff6937c109 _pthread_start + 148 9 libsystem_pthread.dylib 0x00007fff69377b8b thread_start + 15 Comment on attachment 438425 [details]
Patch
EWS is angry
Created attachment 438521 [details]
Patch
Created attachment 438538 [details]
Patch
http://wpt.live/encoding/encodeInto.any.sharedworker.html asserts with this change, and I'm not sure why. ASSERTION FAILED: !vm().currentThreadIsHoldingAPILock() and doing this doesn't fix it + m_vm->apiLock().lock(); m_vm->notifyNeedTermination(); + m_vm->apiLock().unlock(); (In reply to Alex Christensen from comment #6) > http://wpt.live/encoding/encodeInto.any.sharedworker.html asserts with this > change, and I'm not sure why. > > ASSERTION FAILED: !vm().currentThreadIsHoldingAPILock() > > and doing this doesn't fix it > > + m_vm->apiLock().lock(); > m_vm->notifyNeedTermination(); > + m_vm->apiLock().unlock(); It's the opposite, we should not be holding the lock when calling notifyNeedTermination(). I think this means that the worker thread gets marked for termination while in the middle on running script. I don't have access to the full backtrace but I bet that you'll see that it's running script or doing some other operation that holds the JS lock. hi Alex, trust this finds you well. Is there any update on this ticket please? Also interested in any updates. I'm currently working on a feature that requires sharedworkers. (In reply to Daniel St-Germain from comment #10) > Also interested in any updates. I'm currently working on a feature that > requires sharedworkers. I am actively working on this (via dependency bugs). This is now tracked as: <rdar://64348204> *** Bug 149850 has been marked as a duplicate of this bug. *** It is actually exciting to see that this ticket is flagged as resolved now :) In which Safari Tech Preview is the SW support going to land? In case you need a really advanced real world test-case: https://neomjs.github.io/pages/node_modules/neo.mjs/apps/sharedcovid/index.html#mainview=table Be aware though that the app will fall back to using non shared workers if needed. In case you can move components like the chart, helix or gallery into new browser windows, it is a success. Demo video (Chrome): https://youtu.be/aEA5333WiWY (starts at 2:40). Thanks and best regards, Tobias (In reply to Tobias Uhlig from comment #14) > It is actually exciting to see that this ticket is flagged as resolved now :) > > In which Safari Tech Preview is the SW support going to land? > > In case you need a really advanced real world test-case: > https://neomjs.github.io/pages/node_modules/neo.mjs/apps/sharedcovid/index. > html#mainview=table > > Be aware though that the app will fall back to using non shared workers if > needed. In case you can move components like the chart, helix or gallery > into new browser windows, it is a success. > > Demo video (Chrome): > https://youtu.be/aEA5333WiWY (starts at 2:40). > > Thanks and best regards, > Tobias It usually takes a few weeks to get into an STP build. We will update this bug once the relevant STP build is available for download, |