Conversation
0aec773 to
47f23e6
Compare
|
I think this is ready for review! Looks like there might be a bug somewhere because the test harness has gotten stuck on (some) Linux jobs, it seems.
Edit: I found a way around for WASI. There is some pthreads compatibility in the WASI SDK I had missed! |
bnoordhuis
left a comment
There was a problem hiding this comment.
Not sure why test262 hangs. It looks like it should Just Work.
f523443 to
a538568
Compare
|
Alright, new strategy. I disabled atomics on Emscripten / WASI, but kept the bytecode compatible by always having the atoms defined. Threading support is still experimental on wasmtime, so I guess WASI can wait. Once tests pass this should be good to go. More Windows portability will follow to enable workers, but that is in the libc, I'm focusing on the engine first. |
| } | ||
| ret = pthread_cond_timedwait(&waiter->cond, &js_atomics_mutex, | ||
| &ts); | ||
| ret = js_cond_timedwait(&waiter->cond, &js_atomics_mutex, timeout * 1e6 /* to ns */); |
There was a problem hiding this comment.
@bnoordhuis This was the reason for the hang. Timeot is in ms for Atomics.wait but in the C wrapper I take ns like libuv 😅
|
And we are 💚 -- @bnoordhuis PTAL! |
| } BCTagEnum; | ||
|
|
||
| #define BC_VERSION 9 | ||
| #define BC_VERSION 10 |
There was a problem hiding this comment.
For my understanding, is the bump necessary because the atom ids change when the #ifdef CONFIG_ATOMICS is removed from quickjs-atom.h?
There was a problem hiding this comment.
Correct. That's the source of the bytecode incompatibility.
Fixes: #1