Closed Bug 1231333 Opened 4 years ago Closed 4 years ago

Disallow Atomics.futexWait on the main thread

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: lth, Assigned: lth)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

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.
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.
Make futexWait throw a TypeError if waiting is disabled on the runtime, and create an API to opt in to futexWaiting.
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Attachment #8701450 - Flags: review?(luke)
Using the new JS engine API, opt in to futexWaiting on plain WebWorker threads only.
Attachment #8701452 - Flags: review?(khuey)
Attachment #8701450 - Flags: review?(luke) → review+
Comment on attachment 8701452 [details] [diff] [review]
bug1231333-no-futexWait-part2.patch

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

Braces please.
Attachment #8701452 - Flags: review?(khuey) → review+
Jukka: FYI, this is about to land.
Flags: needinfo?(jujjyl)
https://hg.mozilla.org/mozilla-central/rev/d16977f0dd8f
https://hg.mozilla.org/mozilla-central/rev/2a2a7d610bc7
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
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?
Flags: needinfo?(jujjyl) → needinfo?(lhansen)
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.
Flags: needinfo?(lhansen)
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
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.
You need to log in before you can comment on or make changes to this bug.