1231333 - Disallow Atomics.futexWait on the main thread

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Lars T Hansen [:lth]]

For the time it should not be allowed to futexWait on the main thread, as this is controversial and will probably deadlock the browser (due to internal architectural issues).  Exactly how it will be disallowed (throwing an exception, returning an error code) is a spec issue that remains to be resolved; either is easy to implement.

Comment 1

[Lars T Hansen [:lth]]

The better way to think about this is that futexWait should be disallowed globally - it throws a TypeError - and then appropriate agent types opt in to it.  WebWorkers would opt in, but SharedWorkers and ServiceWorkers would not (yet) do so (see bug 1232973 for justification / discussion), and the main thread would not, or maybe it would opt in in a different way, such as allowing micro-waits.

Comment 2

[Lars T Hansen [:lth]]

Attachment: bug1231333-no-futexWait-part1.patch

Make futexWait throw a TypeError if waiting is disabled on the runtime, and create an API to opt in to futexWaiting.

Comment 3

[Lars T Hansen [:lth]]

Attachment: bug1231333-no-futexWait-part2.patch

Using the new JS engine API, opt in to futexWaiting on plain WebWorker threads only.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8701452 [details] [diff] [review]
bug1231333-no-futexWait-part2.patch

Review of attachment 8701452 [details] [diff] [review]:
-----------------------------------------------------------------

Braces please.

Comment 5

[Lars T Hansen [:lth]]

Jukka: FYI, this is about to land.

Comment 6

[Lars T Hansen [:lth]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d16977f0dd8f489f16f1429a7f62ff7bae6121bf
Bug 1231333 - part 1, JS engine: only allow futexWait in workers. r=luke

https://hg.mozilla.org/integration/mozilla-inbound/rev/2a2a7d610bc74284c37c9c36b5d47d4ffa6230fb
Bug 1231333 - part 2, DOM: only allow futexWait in workers.  r=khuey

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d16977f0dd8f
https://hg.mozilla.org/mozilla-central/rev/2a2a7d610bc7

Comment 8

[Jukka Jylänki]

Thanks for the heads up. I'm now testing the effects of this in practice, and based on comment 2 I was expecting Atomics.futexWait() on the main thread to throw an exception, but in my tests, it still works on Windows Nightly 46.0a1 (2016-01-07). Lars, can you confirm this?

I'm looking at the API to opt in and trying to understand how it behaves. Does our browser main thread (still) opt in?

Comment 9

[Lars T Hansen [:lth]]

The latest Nightly appears to have been built without that patch - I tested on Mac OS X.  A version I built locally from current sources has the patch as expected.

You can run this test case:

http://lars-t-hansen.github.io/ecmascript_sharedmem/test-html/test_futexMainWakeup.html

When you click on the button it will test whether futexWait is blocked on the main thread, and give you a clear message about that if it is.

Comment 10

[Jukka Jylänki]

Ah, thanks! Building mozilla-central locally, I see it as expected. I suppose Nightly was not yet up to date with that change, though it was two days after :o

Comment 11

[Jukka Jylänki]

This is being now adapted to in https://github.com/kripken/emscripten/pull/4024 . It looks like eventually we are looking at migrating to a model where the browser main thread will not run the main thread of the Emscripten page, so the need for this kind of emulation will become insignificant.