Fix race in SAB#grow

[syg]

Closes #39

Some interesting tidbits:

• HostGrowSharedArrayBuffer does its own bounds checking so the
  host can control when to do the read as part of a LL/SC pair
• The zeroing semantics basically requires ZFOD pages

Also cc @conrad-watt for review, not sure why I can't enter the handle on the reviewers column.

[conrad-watt]

I think this is based on my comment in the issue? I wrote this because I believed that on failure, we still perform a read of the current length. In fact, this isn't true (we emit no event in the failure case), so the emission of the axiomatic event has more in common with Atomics.add iiuc.

edit: I misread that the API takes an absolute new length, so i should have said Atomics.exchange.

[syg]

Maybe I'm still misunderstanding, since there is a compare still for if the new length is legal and hasn't been changed since it was read. Wouldn't an exchange not guard the OP's issue in #39, where a race can result in an accidental shrink?

(I've never manually programmed LL/SC, so I was perhaps incorrectly thinking LL/SC can actually express the CAS here where the compare isn't just a ==.)

Edit: Stray words.

[conrad-watt]

Ah, I think I see the source of my confusion. The test in line 319 (If new < current then throw) can succeed/throw entirely non-deterministically, since _currentByteLength_ is picked non-deterministically, and if the operation actually throws in this way, no event is currently emitted recording the read value of _currentByteLength_. There's a similar issue with the If new == current test to skip the emission of the RMW.

In Wasm, growth failure is just entirely non-deterministic (even prior to shared memory), so the Wasm specification looks like

on growth

• either fail (with no distinction as to whether OOB or OOM error), or
• non-deterministically pick a current length such that current+growth <= max and emit the corresponding RMW event

Here there's some more subtlety. IIUC JS implementations are not required to throw a RangeError on alloc failure (although they are advised to). Presumably you only want the spec to mandate throwing a RangeError if new < current. In any case, you also want to only be idempotent if new == current.

I think the simplest solution is to make sure that this operation always emits at least an SC read event if it fails either due to the new < current check, or skips due to the new = current check. It's still permitted (I guess) for the operation to emit nothing if there's an OOM error (which isn't specified explicitly).

TL;DR The current alg does not emit a read event on OOB failure, meaning it's more non-deterministic than it should be. If the alg were to emit a read event on failure, a parallel could be drawn with Atomics.compareExchange.

[syg]

I see, thanks for spelling out the thinking process here, very helpful.

Emitting an SC read on failure sounds fine to me.

As for are OOMs mandated to throw, my intention was yes but this was lost for the SAB case because of the spec fiction that SABs allocate a maxLength shared data block up front. ABs have this point because the spec mechanism is creating a new data block and copying over the old contents. I wonder what the best thing to do here is to convey that OOM errors must throw...

[conrad-watt]

LGTM (concentrating on the relaxed memory semantics)

[marjakh]

lgtm (the github ui confuses me to no end, why isn't there a lgtm button in this view? where is it!!1+??)

[marjakh]

Aha, I need to go to the diff view to find the button.