Bug 230382 - Implement SharedWorker
Summary: Implement SharedWorker
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
: 149850 (view as bug list)
Depends on: 233769 233772 233793 233848 233895 235958 236271 236302 236317 236319 236329 236334 236340 236396 236411 236419
Blocks:
  Show dependency treegraph
 
Reported: 2021-09-16 18:12 PDT by Alex Christensen
Modified: 2022-03-21 02:59 PDT (History)
29 users (show)

See Also:


Attachments
Patch (88.92 KB, patch)
2021-09-16 18:15 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (570.71 KB, patch)
2021-09-17 15:08 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (582.38 KB, patch)
2021-09-17 17:56 PDT, Alex Christensen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2021-09-16 18:12:21 PDT
Implement SharedWorker
Comment 1 Alex Christensen 2021-09-16 18:15:23 PDT
Created attachment 438425 [details]
Patch
Comment 2 Geoffrey Garen 2021-09-17 08:09:01 PDT
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 3 Geoffrey Garen 2021-09-17 08:09:15 PDT
Comment on attachment 438425 [details]
Patch

EWS is angry
Comment 4 Alex Christensen 2021-09-17 15:08:41 PDT
Created attachment 438521 [details]
Patch
Comment 5 Alex Christensen 2021-09-17 17:56:25 PDT
Created attachment 438538 [details]
Patch
Comment 6 Alex Christensen 2021-09-17 17:57:41 PDT
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();
Comment 7 Radar WebKit Bug Importer 2021-09-23 18:13:30 PDT
<rdar://problem/83474485>
Comment 8 Chris Dumez 2021-09-27 10:35:32 PDT
(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.
Comment 9 zhenya 2021-11-12 02:39:38 PST
hi Alex, trust this finds you well. Is there any update on this ticket please?
Comment 10 Daniel St-Germain 2021-11-28 19:18:28 PST
Also interested in any updates. I'm currently working on a feature that requires sharedworkers.
Comment 11 Chris Dumez 2022-02-01 11:36:35 PST
(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).
Comment 12 Brent Fulgham 2022-02-08 11:18:32 PST
This is now tracked as:
<rdar://64348204>
Comment 13 Brent Fulgham 2022-02-15 11:28:45 PST
*** Bug 149850 has been marked as a duplicate of this bug. ***
Comment 14 Tobias Uhlig 2022-02-15 11:56:12 PST
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
Comment 15 Brent Fulgham 2022-02-15 13:04:16 PST
(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,