Skip to content

Set current thread before lowering stream/future reads#12736

Draft
TartanLlama wants to merge 1 commit intobytecodealliance:mainfrom
TartanLlama:sy/future-stream-thread
Draft

Set current thread before lowering stream/future reads#12736
TartanLlama wants to merge 1 commit intobytecodealliance:mainfrom
TartanLlama:sy/future-stream-thread

Conversation

@TartanLlama
Copy link
Contributor

The current thread is not set before lowering the results of stream/future reads, which can potentially call realloc.

Putting this up as a draft for now as I'm off tomorrow, will add some tests for this on Monday.

@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Mar 5, 2026
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Longer-term (@dicej you may have thoughts on this too) I feel like the juggling here is pretty brittle and it seems like we should have a new argument to LowerContext::new or something like that. I'm not sure if that would work well given all the various juggling involved, but might be something worth considering.

mut store: StoreContextMut<T>,
caller: RuntimeComponentInstanceIndex,
caller_instance: RuntimeComponentInstanceIndex,
caller_thread: QualifiedThreadId,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this argument is always self.concurrent_state_mut().current_guest_thread()?, so could the argument be dropped and that inlined directly below?

let id = TableId::<TransmitHandle>::new(rep);
log::trace!("copy values {values:?} for {id:?}");

store.0.set_thread(read_caller_thread)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind leaving comments here, and above, for why the current thread is set?

Also, is this something where we want to restore the previous thread after this finishes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wasmtime:api Related to the API of the `wasmtime` crate itself

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants