WebAssembly / threads

Threads and Atomics in WebAssembly
https://webassembly.github.io/threads/
Other
702 stars 50 forks source link

Data initialization synchronization #96

Open conrad-watt opened 6 years ago

conrad-watt commented 6 years ago

The current proposal has this to say about instantiation.

Data segments are initialized in their definition order

  • From low to high bytes
  • At byte granularity (which can be coalesced)
  • As non-atomics
  • An entire module's data section initialization then synchronizes with other operations (effectively, followed by a barrier)

I have two questions/observations/conversation starters about these restrictions, the second of which mainly applies to bulk memory operations (which, regardless of which solution we choose for https://github.com/WebAssembly/threads/issues/94, should have the same semantics for filling order).

  1. I'm not clear on what the intention of the last restriction is. I don't think this should be here, since I've separately been told that bulk memory operations will not be thread safe, and a barrier at the end of the initialization doesn't seem to guarantee much anyway - if you're racing with the initialization, a barrier at the very end won't help you, and if you don't want to race, waiting for the initialization to finish implies a synchronizes-with edge anyway (through a wait/wake call or atomic busy wait).

  2. The specification that segments are initialized in order from low to high bytes is not observable, since the individual writes themselves are non-atomic (and could be observed re-ordered). It's likely still convenient to specify it this way, but it's worth noting that this still allows an implementation to fill from high to low bytes, as a way of efficiently implementing a bounds-check using trap handlers.

binji commented 6 years ago

I believe I added this at the suggestion of @jfbastien, perhaps he can explain the rationale for these lines.

jfbastien commented 6 years ago

Sorry for the late reply, I just came back from vacation.

  1. The idea with having a barrier at the end is that you can load modules from different threads and have some semblance of ordering. I agree that the user needs to synchronize to rely on it anyways, but realistically we'll be doing sys calls before / after a module is loaded which means that there effectively are barriers there anyways. Might as well guarantee it, even if it doesn't provide much.

  2. Right, it's not observable but it's a small thing that we can do to be slightly less hostile. There's little point in allowing implementation leeway here. I don't understand the trap thing you mention: all writes have to occur up to the trap, so it kinda simplifies things to specify low-to-high.

conrad-watt commented 6 years ago

My own apologies too!

re 1. I'll have to think about what this would mean for the formal model in more detail. If you've toyed with any examples/litmus tests that benefit from the barrier, especially in the instantiation case, please pass them on to me!

re 2. "all writes have to occur up to the trap" - the current specification for bulk memory ops has them as all-or-nothing, in the sense that an ahead of time bounds check is (abstractly) used to ensure that the write will either entirely succeed or trap with no effect. That being said, is this feasible to implement in the case of a multi-page fill in the presence of a racy mem.protect on one of the pages...?

Since mem.protect has been the catalyst for a couple of different gotcha moments so far, is there a championed proposal for it knocking around? Since you first flagged it up to me, I've been assuming it acts like the linux mprotect syscall, at a granularity of Wasm-pages.

jfbastien commented 6 years ago
  1. I don't have a litmus test for this.
  2. That seems wrong, @binji ? I thought the intent for bulk memory was to match segment initialization, and we decided long ago to perform all writes from low to high address until OOB? Or I'm confused :)

I think there's only been interest in doing mem.protect in the future. @lukewagner @lars-t-hansen and others had discussed it, IIRC we all agree we'll want it, but it's tricky on the implementation.

conrad-watt commented 6 years ago

Segment initialization, too, is preceded by a bounds check that ensures OOB can't happen half way through. That being said, there are now plans to remove this bounds check, specifically because of the interaction with threads and mem.protect. Maybe there's an analogous discussion that needs to happen about bulk ops.

https://github.com/WebAssembly/spec/pull/820

When I was looking through prior discussions on the bounds check in instantiation, I saw this: https://github.com/WebAssembly/spec/pull/399

I should clarify that I'm getting my bulk memory semantics from this in-progress PR https://github.com/WebAssembly/bulk-memory-operations/pull/19

EDIT: accidently closed issue, sorry

binji commented 6 years ago

Yeah, I was writing it as all-or-nothing in the bulk memory spec to match the current behavior, with the assumption that the threads spec would change it. But you're right, we discussed relaxing the OOB check in the bulk-memory-spec, so that in-progress PR is not correct.

In either case, @jfbastien is right, we want the behavior of the bulk memory ops to match "active" segment initialization too.