Stream: git-wasmtime

Topic: wasmtime / issue #5189 c-api: add wasi_config_set_stdout_...

Wasmtime GitHub notifications bot (Nov 03 2022 at 20:37):

github-actions[bot] commented on issue #5189:

Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "wasmtime:c-api"

Thus the following users have been cc'd because of the following labels:

• peterhuene: wasmtime:c-api

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Nov 04 2022 at 18:08):

ShuP1 commented on issue #5189:

Good points. I was planning to add a pipe in a later PR.
Basically a VecDeque<u8> with read/write interface both for Wasi and API.

In my use case, the "read" API is perfect and easy to implement since wasi_common::pipe are Arc<RwLock<T>> anyway.
It's not a "superset" since Vec is unbounded. A program allocating GBs by flooding stdout is problematic IMO.

Still WIP: https://github.com/bytecodealliance/wasmtime/compare/main...ShuP1:wasmtime:c-api-stdio-pipe-example

In conclusion, this PR can probably be replaced by the next proposal with a capacity limit to the queue.

Wasmtime GitHub notifications bot (Nov 04 2022 at 18:11):

ShuP1 edited a comment on issue #5189:

Good points. I was planning to add a pipe in a later PR.
Basically a VecDeque<u8> with read/write interface both for Wasi and API.

In my use case, the "read" API is perfect and easy to implement since wasi_common::pipe are Arc<RwLock<T>> anyway.
It's not a "superset" since Vec is unbounded. A program allocating GBs by flooding stdout is problematic IMO.

Still WIP: https://github.com/ShuP1/wasmtime/compare/c-api-stdio-buffer...ShuP1:wasmtime:c-api-stdio-pipe-example

In conclusion, this PR can probably be replaced by the next proposal with a capacity limit to the queue.

Wasmtime GitHub notifications bot (Nov 04 2022 at 19:47):

alexcrichton commented on issue #5189:

That also seems plausible to me yeah, and the high-output case could possibly be handled with an optional maximum on the pipe buffer?

I think it would be good to document some more the semantics of the pipes, for example:

• Are the pipes safe to span threads?
• What happens when a write hits the capacity threshold?

I might also recommend a few changes such as:

• I'd avoid wasm_byte_vec_t as it's cumbersome to work with and instead use read/write-style buffer passing
• I'd recommend splitting the pipe into halves and taking ownership when inserting it into a wasi_config_t to avoid the same write half being used in two different instances.

Wasmtime GitHub notifications bot (Nov 05 2022 at 09:38):

ShuP1 commented on issue #5189:

Are the pipes safe to span threads?
Safe to move to another thread but may lock.

Or even maybe dead lock during concurrent use ?
This reaches the limit of my understand of rust async...

https://docs.rs/wasi-common/latest/src/wasi_common/pipe.rs.html#112
RwLock is locked in an async function who does not use await.
So no "context switch" ? and no risk for another thread to hung on this lock ?

Wasmtime GitHub notifications bot (Nov 05 2022 at 10:06):

ShuP1 commented on issue #5189:

I'd avoid wasm_byte_vec_t as it's cumbersome to work with and instead use read/write-style buffer passing

Does this point also includes changing that ?

void wasi_config_set_stdin_bytes(wasi_config_t* config, wasm_byte_vec_t* binary);
// to
void wasi_config_set_stdin_bytes(wasi_config_t* config, size_t, const char*);

Wasmtime GitHub notifications bot (Nov 05 2022 at 10:07):

ShuP1 edited a comment on issue #5189:

I'd avoid wasm_byte_vec_t as it's cumbersome to work with and instead use read/write-style buffer passing

Does this point also includes changing that ?

void wasi_config_set_stdin_bytes(wasi_config_t* config, wasm_byte_vec_t* binary);
// to
void wasi_config_set_stdin_bytes(wasi_config_t* config, size_t, const byte_t*);

Wasmtime GitHub notifications bot (Nov 05 2022 at 10:08):

ShuP1 edited a comment on issue #5189:

I'd avoid wasm_byte_vec_t as it's cumbersome to work with and instead use read/write-style buffer passing

Does this point also includes changing that ?

void wasi_config_set_stdin_bytes(wasi_config_t* config, wasm_byte_vec_t* binary);
// to
void wasi_config_set_stdin_bytes(wasi_config_t* config, const byte_t*, size_t);

Last updated: Nov 22 2024 at 16:03 UTC