wapc/wapc-go

Is it intentional that bi-directional communication is partially supported?

Opened this issue · 6 comments

cuongvo commented

Per the waPC specifications it looks like after the guest is invoked initially, the supported flow is the guest is allowed to make 0..N __host_call before returning.

There's another flow, where communication is bi-directional, or where multiple __guest_calls occur from a single invoke:

  • Guest is invoked
  • Guest does some work, and calls host
  • Host starts a new __guest_call
  • ... new flow starts. Guest eventually returns results to host
  • Host returns results to guest
  • Guest returns

This flow currently fails for the final __guest_call in wasmer (assumption is it also fails in wasmtime), but works for wazero. This is due to the use of invokeContext. In wasmer / wasmtime this is a pointer stored on the shared instance and the response copied to it (here), but the return value of invoke is read from the original invokeContext (here). With multiple __guest_call in a single flow, the shared context is replaced. The output is copied to the invokeContext of the previous __guest_call and the final __guest_call in the chain returns a zero value.

wazero supports this flow since each __guest_call via invoke using wazero gets its own context, not a shared context on the instance (here).

Whew, with that out of the way. Is the above bi-directional flow supported? If not, should it be supported and the implementation for wasmer and wasmtime changed to allow it?

pkedy commented

Whew, with that out of the way. Is the above bi-directional flow supported? If not, should it be supported and the implementation for wasmer and wasmtime changed to allow it?

Thanks for reporting this and the detailed explanation! This bi-directional communication with several back and forth hops absolutely should be supported. I'll take a look at this.

  • 👍1
codefromthecrypt commented

warning, these are just drive-by thoughts from someone only using go for <1.5years, and also I work on wazero.

I'm not sure any CGO lib would be able to propagate go context.. if anything some pointer magic and the lib would be very aware of it. I've not seen anything in the rust projects (wasmer, wasmtime) that would do it.

An alternative could be to change wapc's callback apis to add a u64 param and require it as a sort of correlation context parameter. you could for example store an unsafe pointer of a context.Context between calls that way, and look it up.

A less api effecting way could be to use something like go-eden for context-storage. When CGO is already in use, it claims to have an efficient mechanism that internally correlates data based on a goroutine ID. If you use that, be careful to not make it a required dep for all wasm runtimes and also watch for supported arch/os because the non-CGO alternative is documented (and looks) expensive https://github.com/go-eden/routine/blob/b65d4d238e09c18ba6470bb5731f514b8068392b/routine_goid.go#L63

hope this helps!

codefromthecrypt commented

@cuongvo would you be ok with contributing (even pasting here) a failing unit test for the flow? One that would go into the main test file?

cuongvo commented

Apologies, just saw this. Sure, I can provide a failing unit test to demonstrate this.

cuongvo commented

@codefromthecrypt Here's a commit that shows the error: d60104a

I removed wasmtime from the engines since I didn't have time to look into getting it running on darwin / arm64. wasmer fails though as expected while wazero passes.

=== RUN   TestBidirectionalEngineCalls
--- FAIL: TestBidirectionalEngineCalls (0.02s)
=== RUN   TestBidirectionalEngineCalls/wasmer
    --- FAIL: TestBidirectionalEngineCalls/wasmer (0.00s)
=== RUN   TestBidirectionalEngineCalls/wasmer/Bidirectional_testing_with_assemblyscript_Guest
        --- FAIL: TestBidirectionalEngineCalls/wasmer/Bidirectional_testing_with_assemblyscript_Guest (0.00s)
=== RUN   TestBidirectionalEngineCalls/wasmer/Bidirectional_testing_with_assemblyscript_Guest/Calls_Function_Bi-Directionally
    engine_test.go:195: Unexpected response message, got , expected aaaaaaaa
            --- FAIL: TestBidirectionalEngineCalls/wasmer/Bidirectional_testing_with_assemblyscript_Guest/Calls_Function_Bi-Directionally (0.00s)



=== RUN   TestBidirectionalEngineCalls/wazero
    --- PASS: TestBidirectionalEngineCalls/wazero (0.01s)
=== RUN   TestBidirectionalEngineCalls/wazero/Bidirectional_testing_with_assemblyscript_Guest
        --- PASS: TestBidirectionalEngineCalls/wazero/Bidirectional_testing_with_assemblyscript_Guest (0.01s)
=== RUN   TestBidirectionalEngineCalls/wazero/Bidirectional_testing_with_assemblyscript_Guest/Calls_Function_Bi-Directionally
            --- PASS: TestBidirectionalEngineCalls/wazero/Bidirectional_testing_with_assemblyscript_Guest/Calls_Function_Bi-Directionally (0.00s)
atifsyedali commented

I hit a similar problem and found that wastime works but wazero and wasmer do not. Additionally, the built-in wasm runtime in browsers also do not exhibit the same problem. I did not run the tests above on wasmtime.