Skip to content

Commit 881ce47

Browse files
authored
Remove erroneous debug assertions about parent tasks (#13041)
The fix in #12570 forgot to remove some stray debug assertions about the parent of a subtask when it's cancelled/dropped. Due to index reuse the assertions happened to pass with the test added in #12570, so the test was updated to "spray" the table a bit to prevent reuse which triggers the assertions. The fix here is then to just remove the assertions as they're no longer necessary. Closes #13024
1 parent db598c4 commit 881ce47

2 files changed

Lines changed: 56 additions & 37 deletions

File tree

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

Lines changed: 9 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3124,7 +3124,7 @@ impl Instance {
31243124
.subtask_remove(task_id)?;
31253125

31263126
let concurrent_state = store.concurrent_state_mut();
3127-
let (waitable, expected_caller, delete) = if is_host {
3127+
let (waitable, delete) = if is_host {
31283128
let id = TableId::<HostTask>::new(rep);
31293129
let task = concurrent_state.get_mut(id)?;
31303130
match &task.state {
@@ -3134,22 +3134,17 @@ impl Instance {
31343134
bail_bug!("invalid state for callee in `subtask.drop`")
31353135
}
31363136
}
3137-
(Waitable::Host(id), task.caller, true)
3137+
(Waitable::Host(id), true)
31383138
} else {
31393139
let id = TableId::<GuestTask>::new(rep);
31403140
let task = concurrent_state.get_mut(id)?;
31413141
if task.lift_result.is_some() {
31423142
bail!(Trap::SubtaskDropNotResolved);
31433143
}
3144-
if let Caller::Guest { thread } = task.caller {
3145-
(
3146-
Waitable::Guest(id),
3147-
thread,
3148-
concurrent_state.get_mut(id)?.ready_to_delete(),
3149-
)
3150-
} else {
3151-
bail_bug!("expected guest caller for `subtask.drop`")
3152-
}
3144+
(
3145+
Waitable::Guest(id),
3146+
concurrent_state.get_mut(id)?.ready_to_delete(),
3147+
)
31533148
};
31543149

31553150
waitable.common(concurrent_state)?.handle = None;
@@ -3164,10 +3159,6 @@ impl Instance {
31643159
waitable.delete_from(concurrent_state)?;
31653160
}
31663161

3167-
// Since waitables can neither be passed between instances nor forged,
3168-
// this should never fail unless there's a bug in Wasmtime, but we check
3169-
// here to be sure:
3170-
debug_assert_eq!(expected_caller, concurrent_state.current_guest_thread()?);
31713162
log::trace!("subtask_drop {waitable:?} (handle {task_id})");
31723163
Ok(())
31733164
}
@@ -3583,25 +3574,12 @@ impl Instance {
35833574
})
35843575
.handle_table()
35853576
.subtask_rep(task_id)?;
3586-
let (waitable, expected_caller) = if is_host {
3587-
let id = TableId::<HostTask>::new(rep);
3588-
(
3589-
Waitable::Host(id),
3590-
store.concurrent_state_mut().get_mut(id)?.caller,
3591-
)
3577+
let waitable = if is_host {
3578+
Waitable::Host(TableId::<HostTask>::new(rep))
35923579
} else {
3593-
let id = TableId::<GuestTask>::new(rep);
3594-
if let Caller::Guest { thread } = store.concurrent_state_mut().get_mut(id)?.caller {
3595-
(Waitable::Guest(id), thread)
3596-
} else {
3597-
bail_bug!("expected guest caller for `subtask.cancel`")
3598-
}
3580+
Waitable::Guest(TableId::<GuestTask>::new(rep))
35993581
};
3600-
// Since waitables can neither be passed between instances nor forged,
3601-
// this should never fail unless there's a bug in Wasmtime, but we check
3602-
// here to be sure:
36033582
let concurrent_state = store.concurrent_state_mut();
3604-
debug_assert_eq!(expected_caller, concurrent_state.current_guest_thread()?);
36053583

36063584
log::trace!("subtask_cancel {waitable:?} (handle {task_id})");
36073585

tests/misc_testsuite/component-model/async/cancel-sibling-subtask.wast

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,16 @@
3737
(import "" "f" (func $f (result i32)))
3838
(import "" "cancel" (func $cancel (param i32) (result i32)))
3939
(import "" "drop" (func $drop (param i32)))
40+
(import "" "future.new" (func $future.new (result i64)))
4041
(global $subtask (mut i32) (i32.const 0))
4142

4243
;; This export starts a call to `$f` in the above component but doesn't
4344
;; await it or complete it. Instead this task exits.
4445
(func (export "a") (param $cancel i32)
4546
(local $ret i32)
4647

48+
(drop (call $future.new))
49+
4750
;; start the subtask
4851
(local.set $ret (call $f))
4952

@@ -81,29 +84,67 @@
8184
(core func $f (canon lower (func $f) async))
8285
(core func $cancel (canon subtask.cancel))
8386
(core func $drop (canon subtask.drop))
87+
(type $ft (future))
88+
(core func $future.new (canon future.new $ft))
8489
(core instance $i (instantiate $m
8590
(with "" (instance
8691
(export "f" (func $f))
8792
(export "cancel" (func $cancel))
8893
(export "drop" (func $drop))
94+
(export "future.new" (func $future.new))
8995
))
9096
))
9197
(func (export "a") async (param "cancel" bool) (canon lift (core func $i "a")))
9298
(func (export "b") async (param "cancel" bool) (canon lift (core func $i "b")))
9399
)
94100

101+
(component $c
102+
(import "b" (instance $b
103+
(export "a" (func async (param "cancel" bool)))
104+
(export "b" (func async (param "cancel" bool)))
105+
))
106+
(core module $m
107+
(import "" "a" (func $a (param i32)))
108+
(import "" "b" (func $b (param i32)))
109+
(import "" "future.new" (func $future.new (result i64)))
110+
111+
(func (export "run") (param i32 i32)
112+
(call $a (local.get 0))
113+
114+
;; push things into the handle table to ensure that b's task is
115+
;; different from a's.
116+
(drop (call $future.new))
117+
118+
(call $b (local.get 1))
119+
)
120+
)
121+
(core func $a (canon lower (func $b "a")))
122+
(core func $b (canon lower (func $b "b")))
123+
(type $ft (future))
124+
(core func $future.new (canon future.new $ft))
125+
(core instance $i (instantiate $m
126+
(with "" (instance
127+
(export "a" (func $a))
128+
(export "b" (func $b))
129+
(export "future.new" (func $future.new))
130+
))
131+
))
132+
133+
(func (export "run") async (param "a" bool) (param "b" bool)
134+
(canon lift (core func $i "run"))
135+
)
136+
)
137+
95138
(instance $a (instantiate $a))
96139
(instance $b (instantiate $b (with "f" (func $a "f"))))
97-
(export "a" (func $b "a"))
98-
(export "b" (func $b "b"))
140+
(instance $c (instantiate $c (with "b" (instance $b))))
141+
(export "run" (func $c "run"))
99142
)
100143

101144
;; start subtask in "a", cancel/drop it in "b"
102145
(component instance $A $A)
103-
(assert_return (invoke "a" (bool.const false)))
104-
(assert_return (invoke "b" (bool.const true)))
146+
(assert_return (invoke "run" (bool.const false) (bool.const true)))
105147

106148
;; start/cancel subtask in "a", drop it in "b"
107149
(component instance $A $A)
108-
(assert_return (invoke "a" (bool.const true)))
109-
(assert_return (invoke "b" (bool.const false)))
150+
(assert_return (invoke "run" (bool.const true) (bool.const false)))

0 commit comments

Comments
 (0)