npmccallum opened PR #3901 from wasi
to main
:
This effort includes two major changes:
Most methods are given a default implementation. This aids
implementors who only need a few of the methods and it allows us to
remove a bunch of boilerplate code.Most methods are changed to take a mutable reference. This is
important since wasmtime cannot know ahead of time what APIs
implementors are trying to adapt.BREAKING CHANGE:
WasiFile
has changed method signaturesSigned-off-by: Nathaniel McCallum <nathaniel@profian.com>
npmccallum updated PR #3901 from wasi
to main
.
npmccallum updated PR #3901 from wasi
to main
.
bjorn3 submitted PR review.
bjorn3 submitted PR review.
bjorn3 created PR review comment:
An argument against this change: Using
&mut self
would require locking on the wasmtime side if multi threading support is ever implemented.&self
allows theWasiFile
impl to avoid locking if it doesn't need it. (eg you can read from and write to an&std::fs::File
without locking.&mut self
would also require double locking in case of a shared resource accessible through multipleWasiFile
s.
bjorn3 created PR review comment:
I think the intention here is that it will eventually be implemented once it becomes possible.
npmccallum submitted PR review.
npmccallum created PR review comment:
@bjorn3 Except that this is an async interface. I'm unware of async-aware locking primitives. And you most certainly don't want to take a lock around an IO operation.
npmccallum submitted PR review.
npmccallum created PR review comment:
@bjorn3 The existing code is inconsistent with regards to mutability. Whatever we do, it should be consistent.
npmccallum submitted PR review.
npmccallum created PR review comment:
@bjorn3 Also,
WasiSnapshotPreview1
takes&mut self
for all operations. That seems to gate this.
npmccallum updated PR #3901 from wasi
to main
.
bjorn3 submitted PR review.
bjorn3 created PR review comment:
I guess so.
npmccallum updated PR #3901 from wasi
to main
.
pchickey submitted PR review.
pchickey merged PR #3901.
Last updated: Dec 23 2024 at 12:05 UTC