Stream: git-wasmtime

Topic: wasmtime / PR #6928 Rework `write` for sockets

Wasmtime GitHub notifications bot (Aug 29 2023 at 20:58):

elliottt requested wasmtime-core-reviewers for a review on PR #6928.

Wasmtime GitHub notifications bot (Aug 29 2023 at 20:58):

elliottt requested alexcrichton for a review on PR #6928.

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:06):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:06):

alexcrichton created PR review comment:

Could this have a comment explaining why this is doing this?

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:06):

alexcrichton created PR review comment:

I think this'll want to be the AbortOnDrop handle (or something like that name) Pat recently added too

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:06):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:21):

alexcrichton created PR review comment:

Here I'd recommend using AsyncWriteExt::write_all since it'll bundle all this internally

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:21):

alexcrichton created PR review comment:

This'll need some special handling to ensure that if this is cancelled that the task isn't forgotten

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:25):

elliottt updated PR #6928.

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:25):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:25):

elliottt created PR review comment:

Great point!

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:29):

elliottt opened PR #6928 from elliottt:trevor/socket-changes to bytecodealliance:pch/backpressure_2:

Spawn a task if we can't immediately satisfy a write, storing its join handle for later. Use the presence of the join handle to decide when writes are ready, or if a flush has completed.

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:30):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:30):

elliottt created PR review comment:

Ah you're right, I should only clear the option if the write task has finished.

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:33):

elliottt updated PR #6928.

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:33):

elliottt updated PR #6928.

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:34):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:36):

alexcrichton created PR review comment:

This may be preexisting, but I think this means that I/O errors turn into trapping errors? Or at least I think that the Err here gets turned into a trap, but I could easily be misremembering

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:36):

elliottt updated PR #6928.

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:50):

elliottt created PR review comment:

From a DM: we'll have to use try_write in a loop as tokio doesn't allow write_all with a non-mutable reference for self.

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:50):

pchickey submitted PR review.

Wasmtime GitHub notifications bot (Aug 29 2023 at 21:53):

pchickey created PR review comment:

The error handling scheme gets fixed in my branch to explicitly call out operation failed vs traps.

Wasmtime GitHub notifications bot (Aug 30 2023 at 23:39):

elliottt updated PR #6928.

Wasmtime GitHub notifications bot (Aug 30 2023 at 23:49):

elliottt updated PR #6928.

Wasmtime GitHub notifications bot (Aug 30 2023 at 23:52):

elliottt updated PR #6928.

Wasmtime GitHub notifications bot (Aug 30 2023 at 23:53):

elliottt created PR review comment:

I've rebased on Pat's branch, and reworked the error handling to categorize error results as traps or runtime errors.

Wasmtime GitHub notifications bot (Aug 30 2023 at 23:53):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Aug 30 2023 at 23:53):

elliottt merged PR #6928.

Last updated: Dec 23 2024 at 12:05 UTC