Skip to content

Commit 1a0f953

Browse files
authored
move waitable to original set after moving it to temporary one (#11357)
There are a few places in `concurrent.rs` where we use `GuestTask::sync_call_set` to wait on waitables during synchronous calls. However, they may have been members of another set before we joined them to `sync_call_set`, in which case we need to move them back when we're done (or at least remove them from `sync_call_set`). Prior to this fix, we would panic when dropping a task which had subtasks which had been synchronously cancelled. I've updated `async_cancel_callee.rs` to cover that case. Signed-off-by: Joel Dice <joel.dice@fermyon.com>
1 parent 4c4e348 commit 1a0f953

File tree

4 files changed

+22
-4
lines changed

4 files changed

+22
-4
lines changed

crates/misc/component-async-tests/tests/scenario/transmit.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ pub async fn async_cancel_caller() -> Result<()> {
4848
test_cancel(Mode::Normal).await
4949
}
5050

51+
#[tokio::test]
52+
pub async fn async_cancel_caller_leak_task_after_cancel() -> Result<()> {
53+
test_cancel(Mode::LeakTaskAfterCancel).await
54+
}
55+
5156
#[tokio::test]
5257
pub async fn async_trap_cancel_guest_after_start_cancelled() -> Result<()> {
5358
test_cancel_trap(Mode::TrapCancelGuestAfterStartCancelled).await

crates/misc/component-async-tests/wit/test.wit

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ interface cancel {
151151
trap-cancel-guest-after-return,
152152
trap-cancel-host-after-return-cancelled,
153153
trap-cancel-host-after-return,
154+
leak-task-after-cancel,
154155
}
155156

156157
run: func(mode: mode, cancel-delay-millis: u64);

crates/test-programs/src/bin/async_cancel_callee.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ const _MODE_TRAP_CANCEL_GUEST_AFTER_RETURN_CANCELLED: u8 = 2;
5757
const _MODE_TRAP_CANCEL_GUEST_AFTER_RETURN: u8 = 3;
5858
const MODE_TRAP_CANCEL_HOST_AFTER_RETURN_CANCELLED: u8 = 4;
5959
const MODE_TRAP_CANCEL_HOST_AFTER_RETURN: u8 = 5;
60+
const MODE_LEAK_TASK_AFTER_CANCEL: u8 = 6;
6061

6162
#[derive(Clone, Copy)]
6263
struct SleepParams {
@@ -166,7 +167,10 @@ unsafe extern "C" fn callback_sleep_with_options_sleep_millis(
166167
}
167168

168169
waitable_join(*waitable, 0);
169-
subtask_drop(*waitable);
170+
171+
if params.mode != MODE_LEAK_TASK_AFTER_CANCEL {
172+
subtask_drop(*waitable);
173+
}
170174

171175
if params.on_cancel_delay_millis == 0 {
172176
match params.on_cancel {

crates/wasmtime/src/runtime/component/concurrent.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2412,8 +2412,10 @@ impl Instance {
24122412

24132413
// Use the caller's `GuestTask::sync_call_set` to register interest in
24142414
// the subtask...
2415+
let guest_waitable = Waitable::Guest(guest_task);
2416+
let old_set = guest_waitable.common(state)?.set;
24152417
let set = state.get_mut(caller)?.sync_call_set;
2416-
Waitable::Guest(guest_task).join(state, Some(set))?;
2418+
guest_waitable.join(state, Some(set))?;
24172419

24182420
// ... and suspend this fiber temporarily while we wait for it to start.
24192421
//
@@ -2468,7 +2470,7 @@ impl Instance {
24682470

24692471
let state = self.concurrent_state_mut(store.0);
24702472

2471-
Waitable::Guest(guest_task).join(state, None)?;
2473+
guest_waitable.join(state, old_set)?;
24722474

24732475
if let Some(storage) = storage {
24742476
// The caller used a sync-lowered import to call an async-lifted
@@ -3167,10 +3169,14 @@ impl Instance {
31673169
if async_ {
31683170
return Ok(BLOCKED);
31693171
} else {
3172+
let waitable = Waitable::Guest(guest_task);
3173+
let old_set = waitable.common(concurrent_state)?.set;
31703174
let set = concurrent_state.get_mut(caller)?.sync_call_set;
3171-
Waitable::Guest(guest_task).join(concurrent_state, Some(set))?;
3175+
waitable.join(concurrent_state, Some(set))?;
31723176

31733177
self.suspend(store, SuspendReason::Waiting { set, task: caller })?;
3178+
3179+
waitable.join(self.concurrent_state_mut(store), old_set)?;
31743180
}
31753181
}
31763182
}
@@ -3945,6 +3951,8 @@ impl Waitable {
39453951
/// remove it from any set it may currently belong to (when `set` is
39463952
/// `None`).
39473953
fn join(&self, state: &mut ConcurrentState, set: Option<TableId<WaitableSet>>) -> Result<()> {
3954+
log::trace!("waitable {self:?} join set {set:?}",);
3955+
39483956
let old = mem::replace(&mut self.common(state)?.set, set);
39493957

39503958
if let Some(old) = old {

0 commit comments

Comments
 (0)