Stream: git-wasmtime

Topic: wasmtime / PR #6823 add a join_background_tasks method to...


view this post on Zulip Wasmtime GitHub notifications bot (Aug 08 2023 at 22:53):

pchickey opened PR #6823 from bytecodealliance:pch/wasi_join_background_tasks to bytecodealliance:main:

This method flushes output and terminates background tasks. Background tasks still terminate as part of Drop!

The problem with the current implementation is that there is no way to wait for output buffered in host background tasks to flush before aborting those tasks as part of dropping the Store/Table. This means that e.g. for a trivial component that prints "hello world\n" to stdout and returns, if the Store&Table drop immediately after execution of the component completes, there is a race and the output may not happen at all.

I don't really love this design, but we got backed into a corner because all of the alternatives we could think up are worse:

Therefore, the only behavior we could come up with was to keep the abort-on-drop behavior for background tasks, and add methods to ensure that background tasks are joined (finished) gracefully. This means that both sync and async users of WasiCtx will need to call the appropriate method to wait on background tasks. This is easy enough for users to miss, but we decided that the alternatives are worse.

Closes https://github.com/bytecodealliance/wasmtime/issues/6811

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

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
-->

view this post on Zulip Wasmtime GitHub notifications bot (Aug 08 2023 at 22:53):

pchickey requested fitzgen for a review on PR #6823.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 08 2023 at 22:53):

pchickey requested wasmtime-core-reviewers for a review on PR #6823.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 08 2023 at 22:54):

pchickey requested alexcrichton for a review on PR #6823.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 10 2023 at 23:44):

alexcrichton updated PR #6823.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 10 2023 at 23:59):

alexcrichton updated PR #6823.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 11 2023 at 08:44):

rylev submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 11 2023 at 08:44):

rylev submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 11 2023 at 08:44):

rylev created PR review comment:

I do wonder if this should also take a timeout just like the sync version. This might encourage users to do the right thing and it has the added benefit of being symetrical with the sync version.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 11 2023 at 08:44):

rylev created PR review comment:

// In order to make sure output is flushed before dropping, use `flush_output`.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 11 2023 at 08:44):

rylev created PR review comment:

Nit: is this necessary if you have to check for the pending op anyway down below?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 11 2023 at 08:44):

rylev created PR review comment:

Should this be sync_flush_output?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 18 2023 at 00:16):

pchickey closed without merge PR #6823.


Last updated: Dec 23 2024 at 13:07 UTC