implement clock_sleep for wasip3

[cpetig]

No description provided.

[cpetig]

This directly poses some questions:

• should block_on subtask_state instead?
• the test fails with
  wasm trap: cannot block a synchronous task before returning
  Did I make a conceptual mistake? Is this a limitation in wasmtime?

[cpetig]

Did I make a conceptual mistake?

Perhaps some remnants of p2 run cause this?

    0: failed to invoke `run` function
    1: error while executing at wasm backtrace:
           0:  0x33a3a - <unknown>!<wasm function 0>
           1:   0x12aa - clock_nanosleep.wasm!wasip3_waitable_set_wait
           2:   0x15ac - clock_nanosleep.wasm!wasip3_subtask_block_on
           3:   0x108a - clock_nanosleep.wasm!clock_nanosleep
           4:    0x824 - clock_nanosleep.wasm!__original_main
           5:    0x6f9 - clock_nanosleep.wasm!_start
           6:  0x332e0 - <unknown>!wasi:cli/run@0.2.6#run

Update: Fixed by using p3 run from #710.

[alexcrichton]

I think for that you'll want to rebase on main to pick up #710

[alexcrichton]

Thoughts on this:

• One possible change to make here is to generate bindings for sync lowerings of these functions rather than async lowerings. That would mean that the C bindings here would be a simple "call the function and return".
• I'd bikeshed the interface of wasip3_subtask_block_on a bit, specifically I don't think it should return anything. Blocking on completion of a subtask is, in theory, an infallible operation. I'd imagine that by calling it you're contractually saying "you now own the subtask, go deal with it". In that sense there's no way the subtask can be cancelled and it's required to finish in the component model.

That would then simplify this where the switch would't be necessary at all, and I believe the loop could also be removed.

As to whether it's reasonable to generate synchronous bindings I'm not entirely sure. For example I don't know if wasi-libc is going to want both synchronous and asynchronous bindings for some functions. If it only ever wants one we can just configure the bindings generator, but if it wants both then we'd either need to hand-write bindings for one or update wit-bindgen to be able to generate bindings for both. I'd personally tend towards though trying to use synchronous bindings over call-then-block-on unless needed otherwise since that'll reduce code size and give more information to the runtime to optimize and such.

[cpetig]

I agree that moving the blocking logic to {the host side|outside of the component} opens up for further optimization. Wit-bindgen C should already be able to generate both sync and async wrappings for imported functions, which still could be compatible even with combining them within a single binary.

I never realized that with cooperative threads you could call into blocking p2 interfaces and it would switch the cooperative task. At least this is what I understood.

So my block_on has been working for me before because I was unknowingly exporting an async run function?

My guess is that calling async functions synchronously will work for all normal libc code (as long as there exists synchronous bindings to write to/read from a stream), but select and poll will need to queue several zero sized reads/writes together with an optional clock_wait_for and then waitable_set.wait . So I feel at one point I will need to call waitable_set.wait from a function called from main. 🤔

[cpetig]

Re block_on:
A state of starting will require waiting and re-issuing the command, so the block_on function would need to return to reissue the clock_wait_for, right? And cancellation would likely require to bubble up in the caller, wouldn't it?

[I didn't handle backpressure and cancellation when I did my initial implementation, so I am not sure I got all details right when I extended the block_on function (e.g. compare to https://github.com/cpetig/wasi-libc/blob/wasip3/libc-bottom-half/sources/wasip3_file_utils.c#L111 ) ]

[alexcrichton]

Wit-bindgen C should already be able to generate both sync and async wrappings for imported functions, which still could be compatible even with combining them within a single binary.

Indeed! What wit-bindgen can't do today, however, is generate both sync/async bindings at the same time for the same function. We could fix that in wit-bindgen though if needed.

I never realized that with cooperative threads you could call into blocking p2 interfaces and it would switch the cooperative task.

There's a bit of subtelty here, so to ensure we're on the same page:

• The WASIp2/WASIp3 distinction doesn't actually mean anything to the component model itself as it's just a bunch of versioned APIs. In terms of runtime component model semantics everything has to do with sync-typed-functions or async-typed-functions
• All functions in WASIp2 are sync-typed-functions (grandfathered in)
• When async-typed-functions are called a coop-threading-enabled world will be able to switch to another thread during that call. This was something I didn't realize eariler on but realize now is effectively why context.{get,set} is going to be used to manage the stack pointer.

Put literally your statement here isn't quite correct because if you call a p2 function (a sync-typed-function) no thread-switching will occur. Once you call a sync-typed-function it's impossible for that task to block at all meaning there's no possible switch points. There's still the case of manual switches but I'm sort of glossing over that for now.

However, also in wasi-libc, the wasip3 target doesn't even have access to WASIp2 headers/functions from a code organizatnio perspective. In that sense everything in wasi-libc will be calling wasip3-native things.

So my block_on has been working for me before because I was unknowingly exporting an async run function?

Correct yeah, it's only possible to call blocking things from async-typed things. That's why tests are currently failing because prior to that the main function was accidentally using the WASIp2 export, a non-async function, meaning that it can't actually block and attempting to traps.

select and poll will need to queue several zero sized reads/writes together with an optional clock_wait_for

Agreed yeah, this is what I was thinking might be the problematic cases.

What I might recommend though for the time being is to adjust the wit-bindgen c call for WASIp3 and make anything sync-lowered as necessary. For example, for this PR, I'd switch these bindings to sync. When we get around to implementing select/poll we can handle the case of needing to call some async functions too. Currently wit-bindgen c doesn't actually support sync-lowered stream/future functions, which is also something we should add...

[alexcrichton]

Oh, also:

A state of starting will require waiting and re-issuing the command, so the block_on function would need to return to reissue the clock_wait_for, right? And cancellation would likely require to bubble up in the caller, wouldn't it?

For "STARTING" there's no need to re-invoke the caller since the task is queued up and waiting, and it's not possible for the callee to cancel itself, just the caller can do that. Given taht there's no need to re-issue anything and since this isn't itself cancelling there's no need to handle cancellation

[cpetig]

So, if I understand correctly, for the caller the difference between starting and started is that for starting you might need to wait on another handle to get to the started (or returned) status and then proceed to waiting on the newly reported subtask handle to get to returned.

(I understood Luke's presentation in a way that starting allocates zero memory on the callee and wrongly assumed that the caller would also lose information, but the callee simply can't reuse the argument storage until the callee received them (non-starting status). Perhaps I remembered an older p3 design.)

[alexcrichton]

Oh it's a bit different yeah. If the callee is in the starting or started states upon returning to the caller the caller will receive this piece of information and unconditionally a subtask handle (both packed into a 32-bit integer). The same subtask handle is then used regardless of when the subtask proceeds from starting to started.

The main place this comes up is during cancellation. Upon successful cancellation if the subtask was in the starting state then the caller still owns all arguments (e.g. resources). If the callee was in the started state the caller no longer owns the resources it passed.

In terms of allocation it is done in reference to the callee. If the callee is in the starting state no arguments have been transferred (so, e.g., no argument areas were allocated or anything like that). Once the callee enters the started state it's all transferred.

[cpetig]

Reading the spec more closely it looks like to proceed after starting from started to returned you wait on the same waitable again, as the code returned by the wait is a data-less enum.

[cpetig]

Could it be reasonable to implement p3 in this less efficient way (call async+waitable-set.wait) and later migrate towards calling sync imports when the path became more clear?

[alexcrichton]

Sounds reasonable to me to handle the sync lowering later yeah, thanks!

[alexcrichton]

Oh oops sorry I missed that these had different names. I thought they were the same which would avoid the need for the #ifdef, my bad!

In any case this is reasonable as is, no need to change further IMO.