Stream: git-wasmtime

Topic: wasmtime / PR #1600 Allow different Handles to act as stdio

view this post on Zulip Wasmtime GitHub notifications bot (Apr 25 2020 at 19:06):

kubkon opened PR #1600 from vfs-stdio to master:

This PR builds upon #1561 and should only be merged after #1516 lands in upstream.

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

view this post on Zulip Wasmtime GitHub notifications bot (Apr 25 2020 at 19:06):

kubkon edited PR #1600 from vfs-stdio to master:

This PR builds upon #1561 and should only be merged after #1561 lands in upstream.

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

view this post on Zulip Wasmtime GitHub notifications bot (Apr 25 2020 at 21:33):

kubkon updated PR #1600 from vfs-stdio to master:

This PR builds upon #1561 and should only be merged after #1561 lands in upstream.

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

view this post on Zulip Wasmtime GitHub notifications bot (Apr 25 2020 at 22:01):

kubkon updated PR #1600 from vfs-stdio to master:

This PR builds upon #1561 and should only be merged after #1561 lands in upstream.

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

view this post on Zulip Wasmtime GitHub notifications bot (Apr 26 2020 at 08:00):

kubkon updated PR #1600 from vfs-stdio to master:

This PR builds upon #1561 and should only be merged after #1561 lands in upstream.

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

view this post on Zulip Wasmtime GitHub notifications bot (Apr 26 2020 at 08:15):

kubkon edited PR #1600 from vfs-stdio to master:

This PR builds upon #1561 and should only be merged after #1561 lands in upstream.

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

Closes #939 (hopefully!)

view this post on Zulip Wasmtime GitHub notifications bot (May 04 2020 at 21:18):

kubkon updated PR #1600 from vfs-stdio to master:

This PR builds upon #1561 and should only be merged after #1561 lands in upstream.

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

Closes #939 (hopefully!)

view this post on Zulip Wasmtime GitHub notifications bot (May 04 2020 at 21:46):

kubkon updated PR #1600 from vfs-stdio to master:

This PR builds upon #1561 and should only be merged after #1561 lands in upstream.

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

Closes #939 (hopefully!)

view this post on Zulip Wasmtime GitHub notifications bot (May 06 2020 at 17:47):

kubkon updated PR #1600 from vfs-stdio to master:

This PR builds upon #1561 and should only be merged after #1561 lands in upstream.

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

Closes #939 (hopefully!)

view this post on Zulip Wasmtime GitHub notifications bot (May 08 2020 at 07:49):

kubkon updated PR #1600 from vfs-stdio to master:

This PR builds upon #1561 and should only be merged after #1561 lands in upstream.

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

Closes #939 (hopefully!)

view this post on Zulip Wasmtime GitHub notifications bot (May 08 2020 at 07:50):

kubkon edited PR #1600 from vfs-stdio to master:

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

Closes #939 (hopefully!)

view this post on Zulip Wasmtime GitHub notifications bot (May 08 2020 at 07:51):

kubkon requested pchickey for a review on PR #1600.

view this post on Zulip Wasmtime GitHub notifications bot (May 08 2020 at 07:51):

kubkon requested pchickey and sunfishcode for a review on PR #1600.

view this post on Zulip Wasmtime GitHub notifications bot (May 21 2020 at 18:40):

kubkon updated PR #1600 from vfs-stdio to master:

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

Closes #939 (hopefully!)

view this post on Zulip Wasmtime GitHub notifications bot (May 21 2020 at 20:05):

peterhuene submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (May 21 2020 at 20:05):

peterhuene created PR Review Comment:

The C API currently only accepts paths to files for stdin/stdout/stderr, so these should probably be OsFile::try_from.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 07 2020 at 06:57):

kubkon updated PR #1600 from vfs-stdio to master:

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

Closes #939 (hopefully!)

view this post on Zulip Wasmtime GitHub notifications bot (Jun 07 2020 at 07:37):

kubkon edited PR #1600 from vfs-stdio to master:

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

Closes #939, #1636, #1735

view this post on Zulip Wasmtime GitHub notifications bot (Jun 08 2020 at 19:34):

peterhuene submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 08 2020 at 19:34):

peterhuene submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 08 2020 at 19:34):

peterhuene created PR Review Comment:

Should this function be merged with create_preview1_instance like create_snapshot0_instance is implemented?

view this post on Zulip Wasmtime GitHub notifications bot (Jun 09 2020 at 11:43):

kubkon updated PR #1600 from vfs-stdio to master:

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

Closes #939, #1636, #1735

view this post on Zulip Wasmtime GitHub notifications bot (Jun 09 2020 at 12:16):

kubkon updated PR #1600 from vfs-stdio to master:

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

Closes #939, #1636, #1735

view this post on Zulip Wasmtime GitHub notifications bot (Jun 09 2020 at 17:15):

kubkon updated PR #1600 from vfs-stdio to master:

There have been requests to allow more than just raw OS handles to act as stdio in wasi-common (see #939). This PR makes this possible by loosening the requirement of the WasiCtxBuilder to accept any type T: Handle + 'static to act as any of the stdio handles.

A couple words about correctness of this approach. Currently, since we only have a single Handle super-trait to represent all possible WASI handle types (files, dirs, stdio, pipes, virtual, etc.), it is possible to pass in any type to act as stdio which can be wrong. However, I envision this being a problem only in the near(est) future until we work out how to split Handle into several traits (FYI @sunfishcode and I have already started figuring this out), each representing a different type of WASI resource. In this particular case, this would be a resource which would implement the interface required for a handle to act as a stdio (with appropriate rights, etc.).

@jeffcharles @pchickey The minimum this PR still needs is an audit of this approach for the time being (whether it's OK to go with it given the potential correctness problems I've outlined above). Docs around different now-public types are also a must if we decide to go with it. Oh, and I haven't really got to testing this approach, i.e., whether this is actually sufficient of a change to facilitate the requirements of #939. Any comments and feedback is thus most welcome!

Closes #939, #1636, #1735

view this post on Zulip Wasmtime GitHub notifications bot (Jun 09 2020 at 17:57):

pchickey submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 09 2020 at 18:19):

kubkon merged PR #1600.

Last updated: Dec 23 2024 at 12:05 UTC