Skip to content

Commit

Permalink
Reserve handle index 0 in the component model
Browse files Browse the repository at this point in the history 
This commit updates the allocation scheme for resources in the component
model to start at 1 instead of 0 when communicating with components.
This is an implementation of WebAssembly/component-model#284.

While this broke a number of tests we have this shouldn't actually break
any components in practice. The broken tests were all overly-precise in
their assertions and error messages and this shouldn't idiomatically
come up in any guest language, so this should not be a practically
breaking change.
  • Loading branch information
@alexcrichton
alexcrichton committed Dec 8, 2023
1 parent 63e2606 commit 4a7cd7d
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 53 deletions.
36 changes: 16 additions & 20 deletions crates/runtime/src/component/resources.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,12 +278,8 @@ impl ResourceTables<'_> {
}

impl ResourceTable {
fn next(&self) -> usize {
self.next as usize
}

fn insert(&mut self, new: Slot) -> u32 {
let next = self.next();
let next = self.next as usize;
if next == self.slots.len() {
self.slots.push(Slot::Free {
next: self.next.checked_add(1).unwrap(),
Expand All @@ -294,36 +290,36 @@ impl ResourceTable {
Slot::Free { next } => next,
_ => unreachable!(),
};
u32::try_from(ret).unwrap()

// The component model reserves index 0 as never allocatable so add one
// to the table index to start the numbering at 1 instead.
u32::try_from(ret + 1).unwrap()
}

fn rep(&self, idx: u32) -> Result<u32> {
match usize::try_from(idx).ok().and_then(|i| self.slots.get(i)) {
// NB: `idx` is decremented by one to account for the `+1` above during
// allocation.
let table_index = idx.checked_sub(1).and_then(|i| usize::try_from(i).ok());
match table_index.and_then(|i| self.slots.get(i)) {
None | Some(Slot::Free { .. }) => bail!("unknown handle index {idx}"),
Some(Slot::Own { rep, .. } | Slot::Borrow { rep, .. }) => Ok(*rep),
}
}

fn get_mut(&mut self, idx: u32) -> Result<&mut Slot> {
match usize::try_from(idx)
.ok()
.and_then(|i| self.slots.get_mut(i))
{
// NB: `idx` is decremented by one to account for the `+1` above during
// allocation.
let table_index = idx.checked_sub(1).and_then(|i| usize::try_from(i).ok());
match table_index.and_then(|i| self.slots.get_mut(i)) {
None | Some(Slot::Free { .. }) => bail!("unknown handle index {idx}"),
Some(other) => Ok(other),
}
}

fn remove(&mut self, idx: u32) -> Result<Slot> {
match usize::try_from(idx).ok().and_then(|i| self.slots.get(i)) {
Some(Slot::Own { .. }) | Some(Slot::Borrow { .. }) => {}
_ => bail!("unknown handle index {idx}"),
};
let ret = mem::replace(
&mut self.slots[idx as usize],
Slot::Free { next: self.next },
);
self.next = idx;
let to_fill = Slot::Free { next: self.next };
let ret = mem::replace(self.get_mut(idx)?, to_fill);
self.next = idx - 1;
Ok(ret)
}
}
12 changes: 6 additions & 6 deletions tests/all/component_model/resources.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ fn mismatch_intrinsics() -> Result<()> {
let ctor = i.get_typed_func::<(u32,), (ResourceAny,)>(&mut store, "ctor")?;
assert_eq!(
ctor.call(&mut store, (100,)).unwrap_err().to_string(),
"unknown handle index 0"
"unknown handle index 1"
);

Ok(())
Expand Down Expand Up @@ -341,7 +341,7 @@ fn drop_guest_twice() -> Result<()> {

assert_eq!(
dtor.call(&mut store, (&t,)).unwrap_err().to_string(),
"unknown handle index 0"
"unknown handle index 1"
);

Ok(())
Expand Down Expand Up @@ -1069,7 +1069,7 @@ fn pass_guest_back_as_borrow() -> Result<()> {

// Should not be valid to use `resource` again
let err = take.call(&mut store, (&resource,)).unwrap_err();
assert_eq!(err.to_string(), "unknown handle index 0");
assert_eq!(err.to_string(), "unknown handle index 1");

Ok(())
}
Expand Down Expand Up @@ -1227,9 +1227,9 @@ fn guest_different_host_same() -> Result<()> {
(import "" "drop2" (func $drop2 (param i32)))
(func (export "f") (param i32 i32)
;; separate tables both have initial index of 0
(if (i32.ne (local.get 0) (i32.const 0)) (then (unreachable)))
(if (i32.ne (local.get 1) (i32.const 0)) (then (unreachable)))
;; separate tables both have initial index of 1
(if (i32.ne (local.get 0) (i32.const 1)) (then (unreachable)))
(if (i32.ne (local.get 1) (i32.const 1)) (then (unreachable)))
;; host should end up getting the same resource
(call $f (local.get 0) (local.get 1))
Expand Down
54 changes: 27 additions & 27 deletions tests/misc_testsuite/component-model/resources.wast
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
(local $r i32)
(local.set $r (call $new (i32.const 100)))

(if (i32.ne (local.get $r) (i32.const 0)) (then (unreachable)))
(if (i32.ne (local.get $r) (i32.const 1)) (then (unreachable)))
(if (i32.ne (call $rep (local.get $r)) (i32.const 100)) (then (unreachable)))

(call $drop (local.get $r))
Expand Down Expand Up @@ -95,13 +95,13 @@

;; resources assigned sequentially
(local.set $r1 (call $new (i32.const 100)))
(if (i32.ne (local.get $r1) (i32.const 0)) (then (unreachable)))
(if (i32.ne (local.get $r1) (i32.const 1)) (then (unreachable)))

(local.set $r2 (call $new (i32.const 200)))
(if (i32.ne (local.get $r2) (i32.const 1)) (then (unreachable)))
(if (i32.ne (local.get $r2) (i32.const 2)) (then (unreachable)))

(local.set $r3 (call $new (i32.const 300)))
(if (i32.ne (local.get $r3) (i32.const 2)) (then (unreachable)))
(if (i32.ne (local.get $r3) (i32.const 3)) (then (unreachable)))

;; representations all look good
(if (i32.ne (call $rep (local.get $r1)) (i32.const 100)) (then (unreachable)))
Expand All @@ -113,7 +113,7 @@
(local.set $r2 (call $new (i32.const 400)))

;; should have reused index 1
(if (i32.ne (local.get $r2) (i32.const 1)) (then (unreachable)))
(if (i32.ne (local.get $r2) (i32.const 2)) (then (unreachable)))

;; representations all look good
(if (i32.ne (call $rep (local.get $r1)) (i32.const 100)) (then (unreachable)))
Expand All @@ -135,13 +135,13 @@
(if (i32.ne (call $rep (local.get $r3)) (i32.const 700)) (then (unreachable)))

;; indices should be lifo
(if (i32.ne (local.get $r1) (i32.const 2)) (then (unreachable)))
(if (i32.ne (local.get $r2) (i32.const 1)) (then (unreachable)))
(if (i32.ne (local.get $r3) (i32.const 0)) (then (unreachable)))
(if (i32.ne (local.get $r1) (i32.const 3)) (then (unreachable)))
(if (i32.ne (local.get $r2) (i32.const 2)) (then (unreachable)))
(if (i32.ne (local.get $r3) (i32.const 1)) (then (unreachable)))

;; bump one more time
(local.set $r4 (call $new (i32.const 800)))
(if (i32.ne (local.get $r4) (i32.const 3)) (then (unreachable)))
(if (i32.ne (local.get $r4) (i32.const 4)) (then (unreachable)))

;; deallocate everything
(call $drop (local.get $r1))
Expand Down Expand Up @@ -241,13 +241,13 @@
(local.set $r2 (call $ctor (i32.const 200)))

;; assert r1/r2 are sequential
(if (i32.ne (local.get $r1) (i32.const 0)) (then (unreachable)))
(if (i32.ne (local.get $r2) (i32.const 1)) (then (unreachable)))
(if (i32.ne (local.get $r1) (i32.const 1)) (then (unreachable)))
(if (i32.ne (local.get $r2) (i32.const 2)) (then (unreachable)))

;; reallocate r1 and it should be reassigned the same index
(call $drop (local.get $r1))
(local.set $r1 (call $ctor (i32.const 300)))
(if (i32.ne (local.get $r1) (i32.const 0)) (then (unreachable)))
(if (i32.ne (local.get $r1) (i32.const 1)) (then (unreachable)))

;; internal values should match
(call $assert (local.get $r1) (i32.const 300))
Expand Down Expand Up @@ -443,7 +443,7 @@
(import "" "ctor" (func $ctor (param i32) (result i32)))

(func $start
(if (i32.ne (call $ctor (i32.const 100)) (i32.const 0)) (then (unreachable)))
(if (i32.ne (call $ctor (i32.const 100)) (i32.const 1)) (then (unreachable)))
)
(start $start)
)
Expand All @@ -464,7 +464,7 @@
(import "" "drop" (func $drop (param i32)))

(func (export "f")
(call $drop (i32.const 0))
(call $drop (i32.const 1))
)
)
(core instance $i (instantiate $m
Expand Down Expand Up @@ -492,10 +492,10 @@
(import "" "drop" (func $drop (param i32)))

(func (export "alloc")
(if (i32.ne (call $ctor (i32.const 100)) (i32.const 0)) (then (unreachable)))
(if (i32.ne (call $ctor (i32.const 100)) (i32.const 1)) (then (unreachable)))
)
(func (export "dealloc")
(call $drop (i32.const 0))
(call $drop (i32.const 1))
)
)
(core instance $i (instantiate $m
Expand Down Expand Up @@ -552,10 +552,10 @@
(import "" "drop" (func $drop (param i32)))

(func (export "alloc")
(if (i32.ne (call $ctor (i32.const 100)) (i32.const 0)) (then (unreachable)))
(if (i32.ne (call $ctor (i32.const 100)) (i32.const 1)) (then (unreachable)))
)
(func (export "dealloc")
(call $drop (i32.const 0))
(call $drop (i32.const 1))
)
)
(core instance $i (instantiate $m
Expand Down Expand Up @@ -617,12 +617,12 @@
(call $drop2 (call $new2 (i32.const 104)))

;; should be referencing the same namespace
(if (i32.ne (call $new1 (i32.const 105)) (i32.const 0)) (then (unreachable)))
(if (i32.ne (call $new2 (i32.const 105)) (i32.const 1)) (then (unreachable)))
(if (i32.ne (call $new1 (i32.const 105)) (i32.const 1)) (then (unreachable)))
(if (i32.ne (call $new2 (i32.const 105)) (i32.const 2)) (then (unreachable)))

;; use different drops out of order
(call $drop2 (i32.const 0))
(call $drop1 (i32.const 1))
(call $drop2 (i32.const 1))
(call $drop1 (i32.const 2))
)

(start $start)
Expand Down Expand Up @@ -701,8 +701,8 @@
(local.set $r2 (call $new2 (i32.const 200)))

;; both should be index 0
(if (i32.ne (local.get $r1) (i32.const 0)) (then (unreachable)))
(if (i32.ne (local.get $r2) (i32.const 0)) (then (unreachable)))
(if (i32.ne (local.get $r1) (i32.const 1)) (then (unreachable)))
(if (i32.ne (local.get $r2) (i32.const 1)) (then (unreachable)))

;; nothing should be dropped yet
(if (i32.ne (call $drops) (i32.const 0)) (then (unreachable)))
Expand Down Expand Up @@ -866,7 +866,7 @@

;; table should be empty at this point, so a fresh allocation should get
;; index 0
(if (i32.ne (call $ctor (i32.const 600)) (i32.const 0)) (then (unreachable)))
(if (i32.ne (call $ctor (i32.const 600)) (i32.const 1)) (then (unreachable)))
)

(start $start)
Expand Down Expand Up @@ -1024,8 +1024,8 @@
(local.set $r1 (call $ctor (i32.const 100)))
(local.set $r2 (call $ctor (i32.const 200)))

(if (i32.ne (local.get $r1) (i32.const 0)) (then (unreachable)))
(if (i32.ne (local.get $r2) (i32.const 1)) (then (unreachable)))
(if (i32.ne (local.get $r1) (i32.const 1)) (then (unreachable)))
(if (i32.ne (local.get $r2) (i32.const 2)) (then (unreachable)))

(call $assert-borrow (local.get $r2) (i32.const 200))
(call $assert-borrow (local.get $r1) (i32.const 100))
Expand Down

0 comments on commit 4a7cd7d

Please sign in to comment.