Stream: git-wasmtime

Topic: wasmtime / PR #10256 Don’t panic when http `handle` funct...

Wasmtime GitHub notifications bot (Feb 20 2025 at 18:03):

primoly requested pchickey for a review on PR #10256.

Wasmtime GitHub notifications bot (Feb 20 2025 at 18:03):

primoly requested wasmtime-core-reviewers for a review on PR #10256.

Wasmtime GitHub notifications bot (Feb 20 2025 at 18:03):

primoly opened PR #10256 from primoly:ret-b4-set to bytecodealliance:main:

wasmtime serve currently presumes that a failure to call response-outparam::set inside handle means the component trapped. But it can also happen by returning before invoking set. This PR accommodates for that.

Wasmtime GitHub notifications bot (Feb 20 2025 at 22:32):

alexcrichton commented on PR #10256:

Thanks! Could you add a test for this as well?

Wasmtime GitHub notifications bot (Feb 21 2025 at 18:37):

primoly updated PR #10256.

Wasmtime GitHub notifications bot (Feb 21 2025 at 18:43):

primoly commented on PR #10256:

I’ve added tests, but they are useless, as they pass even without the changes of this PR. cli_serve_return_before_set is supposed to fail on the old branch. For some reason, when I execute my build of wasmtime it recovers from the panic, but when I use the downloaded prebuilt Wasmtime the program closes.

Here a component that returns before calling set. Invoke with wasmtime serve proxy.wat and then connect to it, for example with curl http://localhost:8080.

proxy.wat

(component
  (type (;0;)
    (instance
      (export (;0;) "incoming-request" (type (sub resource)))
      (export (;1;) "response-outparam" (type (sub resource)))
    )
  )
  (import "wasi:http/types@0.2.0" (instance (;0;) (type 0)))
  (core module (;0;)
    (type (;0;) (func (param i32 i32)))
    (export "cm32p2|wasi:http/incoming-handler@0.2|handle" (func $handle))
    (func $handle (;0;) (type 0) (param i32 i32)
      return
    )
  )
  (core instance (;0;) (instantiate 0))
  (alias export 0 "incoming-request" (type (;1;)))
  (type (;2;) (own 1))
  (alias export 0 "response-outparam" (type (;3;)))
  (type (;4;) (own 3))
  (type (;5;) (func (param "request" 2) (param "response-out" 4)))
  (alias core export 0 "cm32p2|wasi:http/incoming-handler@0.2|handle" (core func (;0;)))
  (func (;0;) (type 5) (canon lift (core func 0)))
  (alias export 0 "incoming-request" (type (;6;)))
  (alias export 0 "response-outparam" (type (;7;)))
  (component (;0;)
    (import "import-type-incoming-request" (type (;0;) (sub resource)))
    (import "import-type-response-outparam" (type (;1;) (sub resource)))
    (import "import-type-incoming-request0" (type (;2;) (eq 0)))
    (type (;3;) (own 2))
    (import "import-type-response-outparam0" (type (;4;) (eq 1)))
    (type (;5;) (own 4))
    (type (;6;) (func (param "request" 3) (param "response-out" 5)))
    (import "import-func-handle" (func (;0;) (type 6)))
    (export (;7;) "incoming-request" (type 0))
    (export (;8;) "response-outparam" (type 1))
    (type (;9;) (own 7))
    (type (;10;) (own 8))
    (type (;11;) (func (param "request" 9) (param "response-out" 10)))
    (export (;1;) "handle" (func 0) (func (type 11)))
  )
  (instance (;1;) (instantiate 0
      (with "import-func-handle" (func 0))
      (with "import-type-incoming-request" (type 6))
      (with "import-type-response-outparam" (type 7))
      (with "import-type-incoming-request0" (type 1))
      (with "import-type-response-outparam0" (type 3))
    )
  )
  (export (;2;) "wasi:http/incoming-handler@0.2.0" (instance 1))
)

Wasmtime GitHub notifications bot (Feb 22 2025 at 00:47):

alexcrichton commented on PR #10256:

I think the difference between binaries is panic=abort as the wasmtime executable has that set in CI which is probably what causes an abort.

For the test here could the output of finish() be inspected to ensure that "panicked" doesn't show up? Or basically assert that it has the expected output? (ideally empty or just a warning or similar)

Wasmtime GitHub notifications bot (Feb 22 2025 at 09:47):

primoly updated PR #10256.

Wasmtime GitHub notifications bot (Feb 24 2025 at 15:34):

alexcrichton submitted PR review.

Last updated: Feb 28 2025 at 03:10 UTC