Dan Gohman (Sep 14 2023 at 21:12): Dan Gohman (Sep 14 2023 at 21:12):Is anyone around who can explain how the bindings module in crates/wasi/src/preview2/mod.rs works?
Alex Crichton (Sep 14 2023 at 21:18):I can try giving it a stab!
Alex Crichton (Sep 14 2023 at 21:19):although as I read it now I fear I may not be of much help
Alex Crichton (Sep 14 2023 at 21:20):oh now I'm thinking it may be to have only some methods async
Alex Crichton (Sep 14 2023 at 21:20):I'm gonna fiddle
Pat Hickey (Sep 14 2023 at 21:36):i could explain it, but im not really around
Pat Hickey (Sep 14 2023 at 21:36):but i can be around :)
Pat Hickey (Sep 14 2023 at 21:36):its convoluted, because when i wrote it we didn't have a way to say "just make some of these generated bindings async, keep the rest sync". i think alex did actually add that recently
Dan Gohman (Sep 14 2023 at 21:37):I got some errors where I needed to use _internal_clocks::wasi::io::poll::Pollable insteaed of _internal_io::wasi::io::poll::Pollable.
Pat Hickey (Sep 14 2023 at 21:38):but basically: for the poll, stream, and filesystem interfaces, we do actually need to do tokio-async things in the methods. (we shouldn't in filesystem, but we do for tokio implementation reasons, to move the blocking calls to a worker thread while the thread running the host call itself can yield)
Pat Hickey (Sep 14 2023 at 21:38):oh
Pat Hickey (Sep 14 2023 at 21:38):hmm.
Pat Hickey (Sep 14 2023 at 21:38):i think this is probably something about that approach that broke when we moved to resources.
Pat Hickey (Sep 14 2023 at 21:38):it used to be those types were "the same" because they were type Pollable = u32 in both modules.
Alex Crichton (Sep 14 2023 at 21:38):I've tried to clean this up a bit in this PR (sorry just came back here)
Pat Hickey (Sep 14 2023 at 21:38):but now theyre unique t ypes in each position
Dan Gohman (Sep 14 2023 at 21:39):ah, that makes sense
Alex Crichton (Sep 14 2023 at 21:39):I think that can be fixed with more stuff specified in with: { ... }
Pat Hickey (Sep 14 2023 at 21:39):the idea is that with should allow you to redirect any types you are use ing
Pat Hickey (Sep 14 2023 at 21:39):yeah
Alex Crichton (Sep 14 2023 at 21:40):my above PR should also fix it since there's only one invocation of bindgen!
Alex Crichton (Sep 14 2023 at 21:40):modulo the sync bits which may need a with: { ... }
Pat Hickey (Sep 14 2023 at 21:41):yes, so the other thing going on in there is we need sync interfaces that dispatch to the async interfaces via preview2's private tokio runtime, in order to support users of wasmtime who are in synchronous rust
Pat Hickey (Sep 14 2023 at 21:41):those should be able to use with
Pat Hickey (Sep 14 2023 at 21:41):thanks for the cleanup PR alex
Dan Gohman (Sep 14 2023 at 21:41):Should read and write be sync, since they're non-blocking?
Pat Hickey (Sep 14 2023 at 21:43):i think read still is technically async for filesystem reads
Pat Hickey (Sep 14 2023 at 21:43):but write can maybe be sync now?
Pat Hickey (Sep 14 2023 at 21:44):filesystem writes got taken care of by the flush changes, but theres still a special case for reads where they need to be done with a blocking syscall because epoll cant do files
Pat Hickey (Sep 14 2023 at 21:44):so they need to get moved to the blocking thread (spawn_blocking)
Dan Gohman (Sep 14 2023 at 21:44):I was under the impression that even read was non-blocking because tokio would read into a buffer on another thread
Pat Hickey (Sep 14 2023 at 21:44):we cant do things that way for... reasons
Pat Hickey (Sep 14 2023 at 21:44):i forget what they are, precisely, at this exact moment
Pat Hickey (Sep 14 2023 at 21:45):but i remember alex and i scratched our heads pretty hard about this earlier this summer and we decided we cant do it
Alex Crichton (Sep 14 2023 at 21:45):it's probably worth revisiting now yeah though
Alex Crichton (Sep 14 2023 at 21:45):in theory I think read/write should be able to be sync methods in rust since they can't block in rust
Alex Crichton (Sep 14 2023 at 21:45):might require some fiddling in the source since I think I just saw an .await in there
Pat Hickey (Sep 14 2023 at 21:46):ok. im gonna step away and let you two take care of revisiting that for now, and go back to http streams
Pat Hickey (Sep 14 2023 at 21:47):but first ill sign off that PR above
Dan Gohman (Sep 14 2023 at 22:09):I cherry-picked that PR into my local branch and it's Working For Me
Dan Gohman (Sep 14 2023 at 22:11):Oh, also, the reason read and write need be async is that those are the file read and write methods, which are basically pread and pwrite, so yeah, they're doing I/O, and should be async in the bindings.
Dan Gohman (Sep 14 2023 at 22:12):So it's all good.
Alex Crichton (Sep 14 2023 at 22:12):I've got a follow-up commit though to remove async from check-write, flush, and write-zeros
Alex Crichton (Sep 14 2023 at 22:13):technically we could make write non-async too but right now bindgen can't differentiate between the fs write function and the io write function
Dan Gohman (Sep 14 2023 at 22:13):ah, I was just figuring that out. These names aren't qualified, so we have some collisions.
Alex Crichton (Sep 14 2023 at 22:14):I can work on improving that tomorrow
Dan Gohman (Sep 14 2023 at 22:22):As a quick heads up, with the bindings cleanup patch, and moving filesystem functions into the resource impl:
error[E0195]: lifetime parameters or bounds on method `advise` do not match the trait declaration
--> crates/wasi/src/preview2/host/filesystem.rs:33:18
|
31 | #[async_trait::async_trait]
| --------------------------- this `where` clause might not match the one in the trait
32 | impl<T: WasiView> HostDescriptor for T {
33 | async fn advise(
| __________________^
34 | | &mut self,
35 | | fd: Resource<types::Descriptor>,
36 | | offset: types::Filesize,
37 | | len: types::Filesize,
38 | | advice: types::Advice,
39 | | ) -> Result<(), types::Error> {
| |_________^ lifetimes do not match method in trait
|
::: crates/wasi/src/preview2/mod.rs:143:7
|
143 | });
| - lifetimes in impl do not match this method in trait
It seems to be expecting advise and all the other functions to be sync, even though they're in the list of async fns
Alex Crichton (Sep 14 2023 at 22:31):oh I think it's because the methods changed
Alex Crichton (Sep 14 2023 at 22:31):you'll probably need them to be [method]descriptor.advise
Alex Crichton (Sep 14 2023 at 22:31):e.g. "[method]descriptor.advise"
Dan Gohman (Sep 14 2023 at 22:40):Where do I use that syntax?
Dan Gohman (Sep 14 2023 at 22:42):Oh, in the only_imports list. That works.
Alex Crichton (Sep 14 2023 at 22:42):ah yeah
Alex Crichton (Sep 14 2023 at 22:42):oh hey that'd solve the namespacing issue too heh
Alex Crichton (Sep 14 2023 at 22:42):albeit indirectly
Dan Gohman (Sep 14 2023 at 22:47):sweet, I got all of the HostDescriptor impl compiling
Dan Gohman (Sep 15 2023 at 18:07):All of wasmtime-wasi now compiles!
Dan Gohman (Sep 15 2023 at 18:09):It appears that wasmtime-wasi-http is pulling in the wasmtime-wasi bindings for polling and streams. Does it make sense for me to jump in and port that code to the new poll interfaces?
Alex Crichton (Sep 15 2023 at 18:23):I'd naively say that whatever passes CI and gets WASI on resources is good enough for now
Alex Crichton (Sep 15 2023 at 18:23):but that's without knowing many specifics
Dan Gohman (Sep 15 2023 at 18:30):wasmtime-wasi-http doesn't compile right now and it looks like the easiest thing might be for me to just upgrade its poll and streams to resources too, so I'm now doing that
Alex Crichton (Sep 15 2023 at 18:31):I'm reading over the PR to early-review
Dan Gohman (Sep 15 2023 at 18:33):I want to make a cleanup pass and hopefully find some way to factor out all those Resource::new_borrow(fd.rep()) things, but it should be in a readable state.
Alex Crichton (Sep 15 2023 at 18:35):yeah I was seeing that -- that's because there's two Descriptor types for example, right?
Alex Crichton (Sep 15 2023 at 18:35):one async and one sync?
Dan Gohman (Sep 15 2023 at 18:35):Yeah, that's one big source of them
Alex Crichton (Sep 15 2023 at 18:36):kk I can try to fix that with more features in bindgen!
Dan Gohman (Sep 15 2023 at 18:36):The other is that Resource doesn't have Clone and sometimes we need to pass the same borrowed resource to multiple calls that need Resource
Alex Crichton (Sep 15 2023 at 18:36):(not for this PR but for later)
Alex Crichton (Sep 15 2023 at 18:36):ok intersting, I hesitate to add Clone as it doesn't seem obviously correct, but it also doesn't seem obviously wrong
Dan Gohman (Sep 15 2023 at 18:36):Yeah, same
Alex Crichton (Sep 15 2023 at 18:37):This is an example of that right?
let input_stream = self
.table_mut()
.push_input_stream_child(input, Resource::<tcp::TcpSocket>::new_borrow(this.rep()))?;
let output_stream = self
.table_mut()
.push_output_stream_child(output, Resource::<tcp::TcpSocket>::new_borrow(this.rep()))?;
yeah
Alex Crichton (Sep 15 2023 at 18:37):kk
Alex Crichton (Sep 15 2023 at 18:38):that's tricky b/c we're probably not managing the actual table entry there?
Alex Crichton (Sep 15 2023 at 18:38):actually, lemme read up a bit on this
Alex Crichton (Sep 15 2023 at 18:40):so question for you, finish-connect: func() -> result<tuple<input-stream, output-stream>, error-code> -- is the intent that finish-connect sort of "consumes self"?
Dan Gohman (Sep 15 2023 at 18:40):No, the socket handle remains live after finish-connect, users need it to do getsockopt/setsockopt-like things
Alex Crichton (Sep 15 2023 at 18:41):mk, but I think it may be a bug still then, where if you call that and then close the original socket?
Alex Crichton (Sep 15 2023 at 18:41):hm I need to read more on .push_input_stream_child
Alex Crichton (Sep 15 2023 at 18:42):hm ok this may need to get updated over time
Alex Crichton (Sep 15 2023 at 18:42):this is the child/parent relationships where you can't destroy the parent if the child is still alive
Alex Crichton (Sep 15 2023 at 18:43):I know we intentionally did this for pollable which is more-or-less a child-like resource though
Alex Crichton (Sep 15 2023 at 18:44):it's sort of beyond component-model semantics but that's not the end of the world
Dan Gohman (Sep 15 2023 at 18:47):When we designed the API, we imagined they'd have reference-counting-like behavior, where you could drop the socket handle and keep the streams live.
Alex Crichton (Sep 15 2023 at 18:49):yeah that seems more appropriate for sockets
Dan Gohman (Sep 15 2023 at 18:49):But right now I'm just trying to work within the existing push_input_stream_child behavior.
Alex Crichton (Sep 15 2023 at 18:49):nah yeah that's fine
Alex Crichton (Sep 15 2023 at 18:49):we may want to change the drop_* methods which delete from the table to reference count in the table itself
Alex Crichton (Sep 15 2023 at 18:49):so for example if you connect and then drop the socket that's ok, but the table entry is kept alive until the two other streams are also dropped and then it's finally actually removed from the table
Alex Crichton (Sep 15 2023 at 18:49):I think we have all the hooks to implement that and shouldn't be too bad (no need to do it in this PR of course)
Dan Gohman (Sep 15 2023 at 19:53):Ok, I did some porting of wasi-http but I'm encountering stuff that I don't know what to do with, so I'm going to ask around for help on those parts.
Eduardo Rodrigues (Sep 15 2023 at 19:59):@Dan Gohman , i believe you can ignore error coming from the legacy modules (this file) as those will be completely removed really soon.
Dan Gohman (Sep 15 2023 at 19:59):Oh, awesome. That should clear up a lot of it.
Pat Hickey (Sep 15 2023 at 21:01):this change looks really great so far, all of the host changes are really simple and mechanical
Dan Gohman (Sep 15 2023 at 23:01):updating the preview1 adapter is... complex. Handles aren't cloneable or even dupable, so a lot of adapter code that just throws around u32 code needs to be restructured
Alex Crichton (Sep 15 2023 at 23:03):One option could be to add an off-by-default option to the rust generator to add Copy and remove the Drop implementation to assist with this
Dan Gohman (Sep 16 2023 at 01:00):arg, so close
Caused by:
unsupported section found in adapter module', crates/test-programs/build.rs:232:10
:upside_down:
Dan Gohman (Sep 18 2023 at 14:55):@Alex Crichton I'm seeing crates/test-programs/wasi-tests/src/bin/sleep.rs fail with "unknown handle index 1048096', crates/test-programs/tests/wasi-preview2-components.rs:284:30". 1048096 happens to be the address of the Pollable in the guest. It looks like a disagreement about the lowering of list<borrow<Pollable>> between the Rust guest bindings where it's an array of &Pollable and the host bindings which appear to want an array of Pollable.
Alex Crichton (Sep 18 2023 at 14:57):got a command to repro?
Alex Crichton (Sep 18 2023 at 14:57):definitely sounds like a bindgen issue
Dan Gohman (Sep 18 2023 at 14:58):on the PR branch, cargo test --features "test-programs/test_programs" --package test-programs sleep
Alex Crichton (Sep 18 2023 at 15:01):hm ok I get a failure
Alex Crichton (Sep 18 2023 at 15:01):not unknown handle index though
Dan Gohman (Sep 18 2023 at 15:02):Do you have my latest branch changes?
Alex Crichton (Sep 18 2023 at 15:03):yeah
Alex Crichton (Sep 18 2023 at 15:03):er, 6dea64a708b25bb1c29216e050ddfeb512d3af29
Alex Crichton (Sep 18 2023 at 15:04):running 1 test
calling subscribe
[crates/test-programs/wasi-tests/src/bin/sleep.rs:14] &p as *const _ = 0x000ffe20
[crates/test-programs/wasi-tests/src/bin/sleep.rs:16] list.as_ptr() = 0x000ffea8
calling poll
test sleep ... FAILED
failures:
---- sleep stdout ----
preopen: TempDir { path: "/var/folders/70/cx6_8hz93jz7kj_km0_5c9_40000gn/T/wasi_components_sleep_ZBEXxT" }
thread 'sleep' panicked at 'called `Result::unwrap()` on an `Err` value: error while executing at wasm backtrace:
0: 0x217824 - wit-component:shim!indirect-wasi:io/poll-poll-list
1: 0x137a - sleep::wasi::io::poll::poll_list::h0c3ff0fbc74b0bb0
at /Users/alex/code/wasmtime/crates/test-programs/wasi-tests/src/bin/sleep.rs:3:1
2: 0x1a68 - sleep::main::h05f297b3af9f1f16
at /Users/alex/code/wasmtime/crates/test-programs/wasi-tests/src/bin/sleep.rs:18:5
3: 0x1ce4 - core::ops::function::FnOnce::call_once::h59e587ee50bf110f
at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/core/src/ops/function.rs:250:5
4: 0x1110 - std::sys_common::backtrace::__rust_begin_short_backtrace::h68fc03414118ad34
at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/std/src/sys_common/backtrace.rs:135:18
5: 0xefe - std::rt::lang_start::{{closure}}::h30274f7edf3867b2
at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/std/src/rt.rs:166:18
6: 0x3b3b - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::h644be9b9b3cfec3d
at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/core/src/ops/function.rs:284:13 - std::panicking::try::do_call::h1664f3cfdc3cc4ba
at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/std/src/panicking.rs:500:40 - std::panicking::try::h92522218794dbe07
at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/std/src/panicking.rs:464:19 - std::panic::catch_unwind::h09082efce4044946
at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/std/src/panic.rs:142:14 - std::rt::lang_start_internal::{{closure}}::h96d2e1cd5d5fb964
error: test failed, to rerun pass `-p test-programs --test wasi-preview2-components`
that's all I'm seeing
Alex Crichton (Sep 18 2023 at 15:04):something looks suppressed by accident
Alex Crichton (Sep 18 2023 at 15:04):I think stdout is being switched to nonblocking
Alex Crichton (Sep 18 2023 at 15:04):b/c I saw a panic about that
Dan Gohman (Sep 18 2023 at 15:06):huh, I'm not sure what's happening
Alex Crichton (Sep 18 2023 at 15:07):intermittently I see
running 1 test
calling subscribe
[crates/test-programs/wasi-tests/src/bin/sleep.rs:14] &p as *const _ = 0x000ffe20
[crates/test-programs/wasi-tests/src/bin/sleep.rs:16] list.as_ptr() = 0x000ffea8
calling poll
test sleep ... FAILED
failures:
---- sleep stdout ----
preopen: TempDir { path: "/var/folders/70/cx6_8hz93jz7kj_km0_5c9_40000gn/T/wasi_components_sleep_7s0B4k" }
thread 'sleep' panicked at 'called `Result::unwrap()` on an `Err` value: error while executing at wasm backtrace:
0: 0x217824 - wit-component:shim!indirect-wasi:io/poll-poll-list
1: 0x137a - sleep::wasi::io::poll::poll_list::h0c3ff0fbc74b0bb0
at /Users/alex/code/wasmtime/crates/test-programs/wasi-tests/src/bin/sleep.rs:3:1
2: 0x1a68 - sleep::main::h05f297b3af9f1f16
at /Users/alex/code/wasmtime/crates/test-programs/wasi-tests/src/bin/sleep.rs:18:5
3: 0x1ce4 - core::ops::function::FnOnce::call_once::h59e587ee50bf110f
at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/core/src/ops/function.rs:250:5
4: 0x1110 - std::sys_common::backtrace::__rust_begin_short_backtrace::h68fc03414118ad34
at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/std/src/sys_common/backerror: io error when listing tests: Os { code: 35, kind: WouldBlock, message: "Resource temporarily unavailable" }
trace.rs:135:18
5: 0xefe - std::rt::lang_start::{{closure}}::h30274f7edf3867b2
at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/std/src/rt.rs:166:18
6: 0x3b3b - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::h644be9b9b3cfec3d
at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/core/src/ops/function.rs:284:13 - std::panicking::try::do_call::h1664f3cfdc3cc4ba
at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/std/src/panicking.rs:500:40 - std::panicking::try::h92522218794dbe07
at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/std/src/panicking.rs:464:19 - std::panic::catch_unwind::h09082efce4044946
at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/std/src/panic.rs:1error: test failed, to rerun pass `-p test-programs --test wasi-preview2-components`
which is rust panicking about stdout being nonblocking I think
Alex Crichton (Sep 18 2023 at 15:07):which would also explain the truncated error
Dan Gohman (Sep 18 2023 at 15:09):I have enabled inherit_stdout and inherit_stderr for that test, just to be able to see the debug prints. Maybe the test is consuming the host process stdout and stderr.
Dan Gohman (Sep 18 2023 at 15:09):diff --git a/crates/test-programs/tests/wasi-preview2-components.rs b/crates/test-programs/tes
ts/wasi-preview2-components.rs
index 7834562b6..1ff6a6ff5 100644
--- a/crates/test-programs/tests/wasi-preview2-components.rs
+++ b/crates/test-programs/tests/wasi-preview2-components.rs
@@ -281,7 +281,7 @@ async fn sched_yield() {
}
#[test_log::test(tokio::test(flavor = "multi_thread"))]
async fn sleep() {
- run("sleep", false).await.unwrap()
+ run("sleep", true).await.unwrap() // FIXME: temporarily enable stdio for debugging
}
#[test_log::test(tokio::test(flavor = "multi_thread"))]
async fn stdio() {
Maybe revert that part, and see if it fixes the test harness stdio?
Alex Crichton (Sep 18 2023 at 15:10):hm no this appears to be macos specific
Alex Crichton (Sep 18 2023 at 15:10):on Linux I get the full error with an unknown handle
Alex Crichton (Sep 18 2023 at 15:11):I'll hunt down the macos bug later
Alex Crichton (Sep 18 2023 at 15:14):ok yeah definitely a bug in bindgen
Alex Crichton (Sep 18 2023 at 15:15):package foo:bar
world foo {
resource x
import y: func(a: list<borrow<x>>)
}
generates:
pub fn y(a: &[&X]) {
#[allow(unused_imports)]
use wit_bindgen::rt::{alloc, string::String, vec::Vec};
unsafe {
let vec0 = a;
let ptr0 = vec0.as_ptr() as i32;
let len0 = vec0.len() as i32;
#[cfg(target_arch = "wasm32")]
#[link(wasm_import_module = "$root")]
extern "C" {
#[link_name = "y"]
fn wit_import(_: i32, _: i32);
}
#[cfg(not(target_arch = "wasm32"))]
fn wit_import(_: i32, _: i32) {
unreachable!()
}
wit_import(ptr0, len0);
}
}
I'd hand-write the bindings for poll_list for now instead
Alex Crichton (Sep 18 2023 at 15:16):while I work on this
Dan Gohman (Sep 18 2023 at 15:17):Ok, I can do that. The adapter already does this, so it's straightforward.
Alex Crichton (Sep 18 2023 at 15:27):mind giving a thumbs up on https://github.com/bytecodealliance/wit-bindgen/pull/669? The fix for this will use some bits there
Dan Gohman (Sep 18 2023 at 15:28):/me looks
Dan Gohman (Sep 18 2023 at 15:31)::thumbs_up:
Alex Crichton (Sep 18 2023 at 17:38):ok I think https://github.com/bytecodealliance/wit-bindgen/pull/670 fixes the issue here
Alex Crichton (Sep 18 2023 at 17:39):although if possible I'd still recommend decoupling that PR from yours
Alex Crichton (Sep 18 2023 at 17:39):and just leave a comment for it to get cleaned up later
Dan Gohman (Sep 18 2023 at 17:42):Thanks!
Dan Gohman (Sep 18 2023 at 17:44):Did you mention earlier that it might be possible to unify the resource types between the sync and async bindings? Right now, running commands on the wasmtime CLI on my branch fails because the stdio stream types now involve resources, and the wasmtime CLI uses the sync bindings, so it's expecting different resource types.
Alex Crichton (Sep 18 2023 at 18:19):can't do that yet but I'm going to work on that next
Dan Gohman (Sep 18 2023 at 18:20):ok, cool, yeah, no rush there
Alex Crichton (Sep 18 2023 at 20:11):ok my truncated error message this morning was https://github.com/bytecodealliance/wasmtime/pull/7058
Dan Gohman (Sep 18 2023 at 21:36):With my latest push, all of test-programs passes!
Dan Gohman (Sep 18 2023 at 21:40):I compiled a wit-bindgen with bytecodealliance/wit-bindgen#670 and copied in the bindings, so I can confirm that that fixes the problem I was seeing.
Alex Crichton (Sep 18 2023 at 21:41):nice
Alex Crichton (Sep 18 2023 at 21:41):once that's in I can publish so it can get pulled in
Alex Crichton (Sep 18 2023 at 22:16):ok wit-bindgen is now published
Alex Crichton (Sep 19 2023 at 17:14):@Dan Gohman I've got some cleanups and various changes to the preview1 adapter, would you like me to PR them or would it be ok to push up to the PR?
Alex Crichton (Sep 19 2023 at 17:14):or alternatively if you're working on this area I'll let you go gangbuster
Dan Gohman (Sep 19 2023 at 17:15):I'm in the middle of trying to reorganize things to fix the &mut Descriptors problem
Alex Crichton (Sep 19 2023 at 17:15):ok I'll wait for that
Alex Crichton (Sep 19 2023 at 17:16):I had a rough idea there which was to use &[RefCell<Descriptor>]
Alex Crichton (Sep 19 2023 at 17:16):and then get_file etc methods would return Result<Ref<'_, File>>
Alex Crichton (Sep 19 2023 at 17:16):I didn't actually implement it fully though
Dan Gohman (Sep 19 2023 at 17:17):Oh, interesting. That might be better than my idea, which was to make descriptors() and descriptors_mut() take a closure that gets passed the Descriptors object.
Alex Crichton (Sep 19 2023 at 17:19):yeah my thinking was that ideally we shy away from all unsafe code and stick to libstd/libcore primitives if possible
Dan Gohman (Sep 19 2023 at 17:20):That's obviously good in theory, but I keep trying to use RefCell in various ways and it keeps not working :-}.
Dan Gohman (Sep 19 2023 at 17:20):But your idea of pushing it down into the array might work out better.
Alex Crichton (Sep 19 2023 at 17:20):nah yeah I know what you mean
Dan Gohman (Sep 19 2023 at 17:22):How would the &[RefCell<Descriptor>] case handle adding a new descriptor to the table?
Alex Crichton (Sep 19 2023 at 17:23):oh I thought we already had unsafe bits for that
Alex Crichton (Sep 19 2023 at 17:23):like a cap pointer which we bumped and ptr::write-ed the new value into
Alex Crichton (Sep 19 2023 at 17:24):ah yeah push is already unsafe in that regard which I think is fine to keep
Alex Crichton (Sep 20 2023 at 18:30):ok I've pushed up configuration of resources in https://github.com/bytecodealliance/wasmtime/pull/7069
Last updated: Oct 23 2024 at 20:03 UTC