Dave Bakker (badeend) (Jan 21 2024 at 21:58): Dave Bakker (badeend) (Jan 21 2024 at 21:58):In the preview2 WASI implementation I'd like the ability for child resources to obtain mutable access to their parent. For most cases I can achieve this by storing a Resource<TParent> in the child resource and then, within the child methods, use self.table_mut() to get a mutable reference to its parent. One roadblock, however, is Subscribe. The current design of that trait prohibits access the ResourceTable from within the ready implementation.
I have a refactor in mind that would enable this, but this will require a change in the signature of Subscribe.
From:
#[async_trait::async_trait]
pub trait Subscribe: Send + 'static {
async fn ready(&mut self);
}
To:
pub trait Subscribe: Send + 'static {
fn poll_ready(&mut self, cx: &mut Context<'_>, table: &mut ResourceTable) -> Poll<()>;
}
Would this be OK?
Pat Hickey (Jan 22 2024 at 17:54):What is the need for Context<'_>?
Dave Bakker (badeend) (Jan 22 2024 at 18:17):What I have in mind is something like this: (abbreviated)
#[async_trait::async_trait]
impl<T: WasiView> poll::Host for T {
async fn poll(&mut self, pollables: Vec<Resource<PollableResource>>) -> Result<Vec<u32>> {
futures::future::poll_fn(move |cx| {
let mut results = Vec::new();
for (pollable, i) in pollables.iter_mut() {
match pollable.poll_ready(cx, table) { // HERE
Poll::Ready(()) => {
results.push(*i);
}
Poll::Pending => {}
}
}
if results.is_empty() {
Poll::Pending
} else {
Poll::Ready(results)
}
})
.await
}
}
Giving access to the table I think would make sense yeah, but not being able to use async would I think be a pretty big thing to overcome for consumers. While it's possible to translate all the existing things to manual state machines it's pretty nice being able to use async.
My knee-jerk to this was "Could &ResourceTable be passed as an argument to the async fn? Given the recent de-Sync-ification though I think that would mean that the returned future wouldn't be Send so that probably wouldn't work.
That being said this is definitely something we've felt, there's internal Arcs/etc to handle this ownership boundary.
Dave Bakker (badeend) (Jan 22 2024 at 18:53):"Could &ResourceTable be passed as an argument to the async fn? Given the recent de-Sync-ification though I think that would mean that the returned future wouldn't be Send so that probably wouldn't work.
Also, then you wouldn't be able to poll more than one pollable. Because the mutable reference would be moved into the first future.
not being able to use async would I think be a pretty big thing to overcome for consumers.
poll_ready is lower level than async so it should be pretty straightforward to build async implementations on top of poll_ready. E.g.
#[async_trait::async_trait]
pub trait Subscribe: Send + 'static {
fn poll_ready(&mut self, cx: &mut Context<'_>, table: &mut ResourceTable) -> Poll<()> {
self.ready().as_mut().poll(cx)
}
async fn ready(&mut self) {}
}
Either within one single trait, or as two separate traits
Alex Crichton (Jan 22 2024 at 18:56):rust doesn't really do the pattern where you must implement at most one method well unfortunately, b/c if callers call ready instead of poll_ready they'll get the wrong behavior if you override poll_ready only
Dave Bakker (badeend) (Jan 22 2024 at 18:57):Yeah, so then as two separate traits
Alex Crichton (Jan 22 2024 at 18:57):I could see something like that working yeha
Alex Crichton (Jan 22 2024 at 18:57):where if you want async you can and if you don't you don't have to
Dave Bakker (badeend) (Jan 22 2024 at 19:00):Roughly:
pub trait Pollable: Send + 'static {
fn poll_ready(&mut self, cx: &mut Context<'_>, table: &mut ResourceTable) -> Poll<()>;
}
#[async_trait::async_trait]
pub trait Subscribe: Send + 'static {
async fn ready(&mut self);
}
where Subscribe is a convenience interface over Pollable
Alex Crichton (Jan 22 2024 at 19:01):yeah that looks reasonable to me
Alex Crichton (Jan 22 2024 at 19:01):and an adapter to make a Pollable from a Subscribe or something like that
Pat Hickey (Jan 22 2024 at 19:24):ok thanks i missed the change from async fn to -> Poll, so yeah monday morning coffee problem
Pat Hickey (Jan 22 2024 at 19:25):i think the design you just came to with alex sounds right?
Pat Hickey (Jan 22 2024 at 19:27):ive found myself wondering if theres some better way to hide the implementation detail of ResourceTable so that we can have much more straightforward references back and forth between children/parents
Alex Crichton (Jan 22 2024 at 19:28):"sure would be nice to have Gc<T>" heh
Pat Hickey (Jan 22 2024 at 19:30):things of that nature
Last updated: Oct 23 2024 at 20:03 UTC