Skip to content

Conversation

rvolosatovs
Copy link
Member

@rvolosatovs rvolosatovs commented Sep 8, 2025

Follow-up to #11440, refs #11632

closes #11631

This makes all the existing p3 test cases pass

@rvolosatovs rvolosatovs requested a review from a team as a code owner September 8, 2025 09:20
@rvolosatovs rvolosatovs requested a review from a team as a code owner September 8, 2025 11:16
@rvolosatovs rvolosatovs requested review from alexcrichton and removed request for a team September 8, 2025 11:16
@rvolosatovs rvolosatovs changed the title refactor(p3-http): use trappable errors p3-http: wasi:[email protected] implementation part 2 Sep 8, 2025
@rvolosatovs rvolosatovs force-pushed the refactor/wasip3-http-trappable branch 2 times, most recently from ee60e0f to ba6d2ad Compare September 8, 2025 11:58
@rvolosatovs rvolosatovs changed the title p3-http: wasi:[email protected] implementation part 2 p3-http: finish wasi:[email protected] implementation Sep 8, 2025
@rvolosatovs rvolosatovs moved this to In review in Ship WASIp3 Sep 8, 2025
@rvolosatovs
Copy link
Member Author

@alexcrichton this is ready for review. I'll address the rest along with the spawned task tracking in yet another follow-up tomorrow (or push here if it's not merged by that time yet)

@rvolosatovs rvolosatovs added this pull request to the merge queue Sep 8, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 8, 2025
Signed-off-by: Roman Volosatovs <[email protected]>
This is something we've been doing in wasip3, but I forgot to port this
over

Signed-off-by: Roman Volosatovs <[email protected]>
Signed-off-by: Roman Volosatovs <[email protected]>
Most importantly this avoids a race condition between `content-length` error observed by `GuestBody`
and hyper I/O driver

Signed-off-by: Roman Volosatovs <[email protected]>
Signed-off-by: Roman Volosatovs <[email protected]>
Signed-off-by: Roman Volosatovs <[email protected]>
@rvolosatovs rvolosatovs force-pushed the refactor/wasip3-http-trappable branch from ded256d to 4ef40a8 Compare September 9, 2025 06:51
@rvolosatovs rvolosatovs added this pull request to the merge queue Sep 9, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 9, 2025
@rvolosatovs rvolosatovs added this pull request to the merge queue Sep 9, 2025
Merged via the queue into bytecodealliance:main with commit f3d7256 Sep 9, 2025
44 checks passed
@rvolosatovs rvolosatovs deleted the refactor/wasip3-http-trappable branch September 9, 2025 08:20
@github-project-automation github-project-automation bot moved this from In review to Done in Ship WASIp3 Sep 9, 2025
@rvolosatovs rvolosatovs self-assigned this Sep 9, 2025
alexcrichton pushed a commit to alexcrichton/wasmtime that referenced this pull request Sep 9, 2025
* refactor(p3-http): use trappable errors

Signed-off-by: Roman Volosatovs <[email protected]>

* feat(p3-http): implement `content-length` handling

Signed-off-by: Roman Volosatovs <[email protected]>

* refactor(p3-http): remove a few resource utilities

Signed-off-by: Roman Volosatovs <[email protected]>

* remove unused test import

Signed-off-by: Roman Volosatovs <[email protected]>

* fix(p3-http): close stream handles on drop

Signed-off-by: Roman Volosatovs <[email protected]>

* test(p3-http): stream responses back

This is something we've been doing in wasip3, but I forgot to port this
over

Signed-off-by: Roman Volosatovs <[email protected]>

* doc(p3-http): add missing docs, internalize more, simplify

Signed-off-by: Roman Volosatovs <[email protected]>

* refactor(p3-http): extract `Body::consume`

Signed-off-by: Roman Volosatovs <[email protected]>

* refactor(p3-http): clean-up `content-length` error reporting

Signed-off-by: Roman Volosatovs <[email protected]>

* refactor(p3-http): drop elided lifetime

Signed-off-by: Roman Volosatovs <[email protected]>

* fix(p3-http): avoid guest body deadlock hazard

Signed-off-by: Roman Volosatovs <[email protected]>

* refactor(p3-http): add more docs, clean-up

Signed-off-by: Roman Volosatovs <[email protected]>

* doc(p3-http): add more docs

Signed-off-by: Roman Volosatovs <[email protected]>

* fix(p3-http): rework result future handling

Most importantly this avoids a race condition between `content-length` error observed by `GuestBody`
and hyper I/O driver

Signed-off-by: Roman Volosatovs <[email protected]>

* add new imports after rebase

Signed-off-by: Roman Volosatovs <[email protected]>

* clean-up `poll_consume`

Signed-off-by: Roman Volosatovs <[email protected]>

* assert content-length `handle` results

Signed-off-by: Roman Volosatovs <[email protected]>

* relax `content_length` test `handle` assert

Signed-off-by: Roman Volosatovs <[email protected]>

---------

Signed-off-by: Roman Volosatovs <[email protected]>
alexcrichton added a commit that referenced this pull request Sep 9, 2025
* support non-async `{stream,future}.cancel-{read,write}` (#11625)

* support non-async `{stream,future}.cancel-{read,write}`

During my earlier stream API refactoring, I had forgotten to support or test
synchronous cancellation; this commit does both.  In the process, I realized the
future API ought to be updated to support blocking cancellation just like the
stream API, so I made that change as well.

This also adds `{Source,Destination}::reborrow` functions, allowing instances of
those types to be reborrowed, such that they may be passed as parameters but
also used again.

Note that I had to move some functions from `impl ConcurrentState` to `impl
Instance` in order to access the store and suspend the current fiber when
synchronously cancelling.

Signed-off-by: Joel Dice <[email protected]>

* reduce code duplication

Signed-off-by: Joel Dice <[email protected]>

---------

Signed-off-by: Joel Dice <[email protected]>

* support and test synchronous `{stream,future}.cancel-{read,write}` (#11645)

* support and test synchronous `{stream,future}.cancel-{read,write}`

Previously, we only supported async calls to those intrinsics; now we support
blocking, synchronous calls as well.

Signed-off-by: Joel Dice <[email protected]>

* update future-read.wast test

Signed-off-by: Joel Dice <[email protected]>

---------

Signed-off-by: Joel Dice <[email protected]>

* p3-http: finish `wasi:[email protected]` implementation (#11636)

* refactor(p3-http): use trappable errors

Signed-off-by: Roman Volosatovs <[email protected]>

* feat(p3-http): implement `content-length` handling

Signed-off-by: Roman Volosatovs <[email protected]>

* refactor(p3-http): remove a few resource utilities

Signed-off-by: Roman Volosatovs <[email protected]>

* remove unused test import

Signed-off-by: Roman Volosatovs <[email protected]>

* fix(p3-http): close stream handles on drop

Signed-off-by: Roman Volosatovs <[email protected]>

* test(p3-http): stream responses back

This is something we've been doing in wasip3, but I forgot to port this
over

Signed-off-by: Roman Volosatovs <[email protected]>

* doc(p3-http): add missing docs, internalize more, simplify

Signed-off-by: Roman Volosatovs <[email protected]>

* refactor(p3-http): extract `Body::consume`

Signed-off-by: Roman Volosatovs <[email protected]>

* refactor(p3-http): clean-up `content-length` error reporting

Signed-off-by: Roman Volosatovs <[email protected]>

* refactor(p3-http): drop elided lifetime

Signed-off-by: Roman Volosatovs <[email protected]>

* fix(p3-http): avoid guest body deadlock hazard

Signed-off-by: Roman Volosatovs <[email protected]>

* refactor(p3-http): add more docs, clean-up

Signed-off-by: Roman Volosatovs <[email protected]>

* doc(p3-http): add more docs

Signed-off-by: Roman Volosatovs <[email protected]>

* fix(p3-http): rework result future handling

Most importantly this avoids a race condition between `content-length` error observed by `GuestBody`
and hyper I/O driver

Signed-off-by: Roman Volosatovs <[email protected]>

* add new imports after rebase

Signed-off-by: Roman Volosatovs <[email protected]>

* clean-up `poll_consume`

Signed-off-by: Roman Volosatovs <[email protected]>

* assert content-length `handle` results

Signed-off-by: Roman Volosatovs <[email protected]>

* relax `content_length` test `handle` assert

Signed-off-by: Roman Volosatovs <[email protected]>

---------

Signed-off-by: Roman Volosatovs <[email protected]>

* p3-http: implementation follow-up (#11649)

* p3: refactor future producers/consumers

Signed-off-by: Roman Volosatovs <[email protected]>

* p3-http: tie lifetime of the spawned task to the bodies

Signed-off-by: Roman Volosatovs <[email protected]>

* p3-http: improve docs

Signed-off-by: Roman Volosatovs <[email protected]>

---------

Signed-off-by: Roman Volosatovs <[email protected]>

* Ignore a wasip3 http test temporarily (#11657)

Filed #11656 to track the eventual resolution.

* don't delete sync-lowered subtasks unless they've exited (#11655)

Previously, we were unconditionally deleting the callee subtask once it returned
a value to a sync-lowered call, but that's only appropriate if the subtask has
exited.  Otherwise, it needs to keep running and only be deleted once it
actually exits.

Thanks to Luke for the `sync-streams.wast` test that uncovered this, which I've
copied from the `component-model` repo.

This also makes a couple of debug logging tweaks that proved useful while
investigating the above issue.

Signed-off-by: Joel Dice <[email protected]>

---------

Signed-off-by: Joel Dice <[email protected]>
Signed-off-by: Roman Volosatovs <[email protected]>
Co-authored-by: Joel Dice <[email protected]>
Co-authored-by: Roman Volosatovs <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

wasip3: HTTP: content-length handling needs filling out
2 participants