Bug 234833 – WebAssembly i32.atomic.wait timeout value incorrectly interpreted by factor 1000

• Status: RESOLVED
• Resolution: FIXED
• Component: WebAssembly

[JP Posma]

Created attachment 448260 [details]
Zipfile containing the same example as on https://github.com/janpaul123/wasm-atomic-safari-bug

When using the i32.atomic.wait instruction in WebAssembly, the timeout value seems to be interpreted by a factor 1000 too much. So, when using 1 second, it waits for 1000 seconds, and using 1 millisecond it waits for 1 second.

Builds tested:
- r287554 on https://webkit.org/build-archives/#mac-bigsur-x86_64%20arm64 (which reports as "Version 15.2 (16612.3.6.1.8, 16612)")
- Safari 15.2 on Big Sur (which is where I discovered this initially)
- Recent versions of Chrome, Firefox, and Edge (it behaves correctly on all those browsers).

It's a bit of work to reproduce, since you need to run the WebAssembly in a Web Worker, using a SharedArrayBuffer (which requires specific headers to be set). So I created a minimal test case, available at https://github.com/janpaul123/wasm-atomic-safari-bug as well as attached in a zip file. See the README.md for how to run it.

Note that using the Javascript equivalent "Atomics.wait" behaves correctly (though be aware that it takes a timeout in milliseconds instead of nanoseconds).

Thanks!
— JP

[JP Posma]

For completeness, here is the spec for this instruction: https://github.com/WebAssembly/threads/blob/main/proposals/threads/Overview.md#wait (not sure if there's a more official W3C version somewhere, too)

Also, props to my teammate Paras Sanghavi for finding this!

[Yusuke Suzuki]

Created attachment 448265 [details]
Patch

[Yusuke Suzuki]

Created attachment 448267 [details]
Patch

[JP Posma]

Thanks for the quick fix Yusuke! That looks good to me.

[Michael Saboff]

Comment on attachment 448267 [details]
Patch

r=me

[EWS]

Committed r287575 (245706@main): <https://commits.webkit.org/245706@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 448267 [details].

[Radar WebKit Bug Importer]

<rdar://problem/87097734>

[Sam Clegg]

We had at least one user report the effect of this bug when using pthreads under emscripten: https://github.com/emscripten-core/emscripten/issues/16499.

Do we know how many versions of safari this bug might have shipped in?   Or maybe which devices were effected?   I'm hoping we can avoid doing some kind of  mitigation (perhaps reverting back to the JS API for some period of time).  Off my a factor 1000 in usleep() is pretty serious.

[Keith Miller]

(In reply to Sam Clegg from comment #8)
> We had at least one user report the effect of this bug when using pthreads
> under emscripten: https://github.com/emscripten-core/emscripten/issues/16499.
> 
> Do we know how many versions of safari this bug might have shipped in?   Or
> maybe which devices were effected?   I'm hoping we can avoid doing some kind
> of  mitigation (perhaps reverting back to the JS API for some period of
> time).  Off my a factor 1000 in usleep() is pretty serious.

I believe this fix shipped in Safari 15.4. So anything before then will have the issue.