Skip to content

Commit 1aafe7c

Browse files
fix(bindgen): track resource origination at runtime
This commit fixes resource origination tracking (i.e. figuring out where a resource was created) that was used when deciding how to borrow a given resource -- the previous code did not work for composed components, and did not/could not properly track inter-component transfers.
1 parent bbc5dba commit 1aafe7c

3 files changed

Lines changed: 90 additions & 72 deletions

File tree

crates/js-component-bindgen/src/intrinsics/mod.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ pub enum Intrinsic {
9292
SymbolAsyncIterator,
9393
SymbolIterator,
9494
ScopeId,
95-
DefinedResourceTables,
9695
HandleTables,
9796

9897
/// Class that conforms to a `ReadableStreams`-like interface and is usable externally
@@ -293,8 +292,6 @@ impl Intrinsic {
293292
",
294293
),
295294

296-
Intrinsic::DefinedResourceTables => {}
297-
298295
Intrinsic::FinalizationRegistryCreate => output.push_str(
299296
"
300297
function finalizationRegistryCreate (unregister) {
@@ -1640,7 +1637,6 @@ impl Intrinsic {
16401637
"base64Compile",
16411638
"clampGuest",
16421639
"ComponentError",
1643-
"definedResourceTables",
16441640
"fetchCompile",
16451641
"finalizationRegistryCreate",
16461642
"getErrorPayload",
@@ -1706,7 +1702,6 @@ impl Intrinsic {
17061702
Intrinsic::Base64Compile => "base64Compile",
17071703
Intrinsic::ClampGuest => "clampGuest",
17081704
Intrinsic::ComponentError => "ComponentError",
1709-
Intrinsic::DefinedResourceTables => "definedResourceTables",
17101705
Intrinsic::FetchCompile => "fetchCompile",
17111706
Intrinsic::FinalizationRegistryCreate => "finalizationRegistryCreate",
17121707
Intrinsic::GetErrorPayload => "getErrorPayload",

crates/js-component-bindgen/src/intrinsics/resource.rs

Lines changed: 50 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
//! Intrinsics that represent helpers that deal with Component Model resources
22
3+
use std::fmt::Write;
4+
35
use crate::intrinsics::{Intrinsic, RenderIntrinsicsArgs};
46
use crate::source::Source;
7+
use crate::uwriteln;
58

69
/// This enum contains intrinsics for supporting Component Model resources
710
#[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq)]
@@ -118,28 +121,32 @@ impl ResourceIntrinsic {
118121
",
119122
),
120123

121-
Self::ResourceTableCreateBorrow => output.push_str(
122-
r#"
123-
function rscTableCreateBorrow(table, rep, scopeId) {
124-
if (scopeId === undefined) {{ throw new Error("missing scopeId"); }}
125-
const free = table[0] & ~T_FLAG;
126-
if (free === 0) {
127-
table.push(scopeId);
128-
table.push(rep);
129-
return (table.length >> 1) - 1;
130-
}
131-
table[0] = table[free << 1];
132-
table[free << 1] = scopeId;
133-
table[(free << 1) + 1] = rep;
134-
return free;
135-
}
136-
"#,
137-
),
124+
Self::ResourceTableCreateBorrow => {
125+
uwriteln!(
126+
output,
127+
r#"
128+
function rscTableCreateBorrow(table, rep, scopeId) {{
129+
if (scopeId === undefined) {{ throw new Error("missing scopeId"); }}
130+
const free = table[0] & ~T_FLAG;
131+
if (free === 0) {{
132+
table.push(scopeId);
133+
table.push(rep);
134+
return (table.length >> 1) - 1;
135+
}}
136+
table[0] = table[free << 1];
137+
table[free << 1] = scopeId;
138+
table[(free << 1) + 1] = rep;
139+
return free;
140+
}}
141+
"#,
142+
);
143+
}
138144

139145
Self::ResourceTableCreateOwn => output.push_str(
140146
"
141147
function rscTableCreateOwn(table, rep) {
142148
const free = table[0] & ~T_FLAG;
149+
table._createdReps.add(rep);
143150
if (free === 0) {
144151
table.push(0);
145152
table.push(rep | T_FLAG);
@@ -196,56 +203,69 @@ impl ResourceIntrinsic {
196203
),
197204

198205
Self::ResourceTransferBorrow => {
206+
let resource_transfer_borrow_fn = self.name();
199207
let handle_tables = Intrinsic::HandleTables.name();
200208
let resource_borrows = Self::ResourceCallBorrows.name();
201209
let rsc_table_remove = Self::ResourceTableRemove.name();
202210
let rsc_table_create_borrow = Self::ResourceTableCreateBorrow.name();
203-
let defined_resource_tables = Intrinsic::DefinedResourceTables.name();
204211
let scope_id = Intrinsic::ScopeId.name();
205-
output.push_str(&format!("
206-
function resourceTransferBorrow(handle, fromTid, toTid) {{
212+
213+
uwriteln!(
214+
output,
215+
r#"
216+
function {resource_transfer_borrow_fn}(handle, fromTid, toTid) {{
207217
const fromTable = {handle_tables}[fromTid];
208218
const fromHandle = fromTable[(handle << 1) + 1];
219+
const toTable = {handle_tables}[toTid] || ({handle_tables}[toTid] = [T_FLAG, 0]);
209220
const isOwn = (fromHandle & T_FLAG) !== 0;
210221
const rep = isOwn ? fromHandle & ~T_FLAG : {rsc_table_remove}(fromTable, fromHandle).rep;
211-
if ({defined_resource_tables}[toTid]) return rep;
212-
const toTable = {handle_tables}[toTid] || ({handle_tables}[toTid] = [T_FLAG, 0]);
222+
223+
if (toTable._createdReps.has(rep)) {{
224+
return rep;
225+
}}
226+
213227
const newHandle = {rsc_table_create_borrow}(toTable, rep, {scope_id});
214228
{resource_borrows}.push({{ rid: toTid, handle: newHandle }});
215229
return newHandle;
216230
}}
217-
"));
231+
"#
232+
);
218233
}
219234

220235
Self::ResourceTransferBorrowValidLifting => {
221236
let handle_tables = Intrinsic::HandleTables.name();
222237
let rsc_table_remove = Self::ResourceTableRemove.name();
223238
let rsc_table_create_borrow = Self::ResourceTableCreateBorrow.name();
224-
let defined_resource_tables = Intrinsic::DefinedResourceTables.name();
225239
let scope_id = Intrinsic::ScopeId.name();
226-
output.push_str(&format!("
240+
output.push_str(&format!(r#"
227241
function resourceTransferBorrowValidLifting(handle, fromTid, toTid) {{
228242
const fromTable = {handle_tables}[fromTid];
243+
const toTable = {handle_tables}[toTid];
229244
const isOwn = (fromTable[(handle << 1) + 1] & T_FLAG) !== 0;
230245
const rep = isOwn ? fromTable[(handle << 1) + 1] & ~T_FLAG : {rsc_table_remove}(fromTable, handle).rep;
231-
if ({defined_resource_tables}[toTid]) return rep;
246+
247+
if (toTable._createdReps.has(rep)) {{
248+
return rep;
249+
}}
250+
232251
const toTable = {handle_tables}[toTid] || ({handle_tables}[toTid] = [T_FLAG, 0]);
233252
return {rsc_table_create_borrow}(toTable, rep, {scope_id});
234253
}}
235-
"));
254+
"#));
236255
}
237256

238257
Self::ResourceTransferOwn => {
239258
let handle_tables = Intrinsic::HandleTables.name();
240259
let rsc_table_remove = Self::ResourceTableRemove.name();
241260
let rsc_table_create_own = Self::ResourceTableCreateOwn.name();
242-
output.push_str(&format!("
261+
output.push_str(&format!(r#"
243262
function resourceTransferOwn(handle, fromTid, toTid) {{
244263
const {{ rep }} = {rsc_table_remove}({handle_tables}[fromTid], handle);
245264
const toTable = {handle_tables}[toTid] || ({handle_tables}[toTid] = [T_FLAG, 0]);
246-
return {rsc_table_create_own}(toTable, rep);
265+
const newHandle = {rsc_table_create_own}(toTable, rep);
266+
return newHandle;
247267
}}
248-
"));
268+
"#));
249269
}
250270

251271
Self::ResourceCallBorrows => {

crates/js-component-bindgen/src/transpile_bindgen.rs

Lines changed: 40 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -323,17 +323,15 @@ pub fn transpile_bindgen(
323323
instantiator.initialize();
324324
instantiator.instantiate();
325325

326-
let mut intrinsic_definitions = source::Source::default();
327-
328-
instantiator.resource_definitions(&mut intrinsic_definitions);
326+
instantiator.resource_definitions();
329327
instantiator.instance_flags();
330328

331329
instantiator.bindgen.src.js(&instantiator.src.js);
332330
instantiator.bindgen.src.js_init(&instantiator.src.js_init);
333331

334332
instantiator
335333
.bindgen
336-
.finish_component(name, files, &opts, intrinsic_definitions);
334+
.finish_component(name, files, &opts, source::Source::default());
337335

338336
let exports = instantiator
339337
.bindgen
@@ -956,7 +954,7 @@ impl<'a> Instantiator<'a, '_> {
956954
}
957955
}
958956

959-
fn resource_definitions(&mut self, definitions: &mut source::Source) {
957+
fn resource_definitions(&mut self) {
960958
// It is theoretically possible for locally defined resources used in no functions
961959
// to still be exported
962960
for resource in 0..self.component.num_resources {
@@ -970,30 +968,21 @@ impl<'a> Instantiator<'a, '_> {
970968
}
971969
}
972970

973-
// Write out the defined resource table indices for the runtime
974-
if self.bindgen.all_intrinsics.contains(&Intrinsic::Resource(
975-
ResourceIntrinsic::ResourceTransferBorrow,
976-
)) || self.bindgen.all_intrinsics.contains(&Intrinsic::Resource(
977-
ResourceIntrinsic::ResourceTransferBorrowValidLifting,
978-
)) {
979-
let defined_resource_tables = Intrinsic::DefinedResourceTables.name();
980-
uwrite!(definitions, "const {defined_resource_tables} = [");
981-
// Table per-resource
982-
for tidx in 0..self.component.num_resources {
983-
let tid = TypeResourceTableIndex::from_u32(tidx);
984-
let resource_table_ty = &self.types[tid];
985-
let rid = resource_table_ty.unwrap_concrete_ty();
986-
if let Some(defined_index) = self.component.defined_resource_index(rid) {
987-
let instance_idx = resource_table_ty.unwrap_concrete_instance();
988-
if instance_idx == self.component.defined_resource_instances[defined_index] {
989-
uwrite!(definitions, "true,");
990-
}
991-
} else {
992-
uwrite!(definitions, ",");
993-
};
994-
}
995-
uwrite!(definitions, "];\n");
996-
}
971+
// TODO(feat): In the past, we could eagerly build a mapping of resources defined to the tables they correspond to
972+
// to make runtime checks of which table a resource must have been created into first (i.e. the component that implements
973+
// creation of a given resource) a particular resource faster.
974+
//
975+
// This logic was based on component.num_resources and was broken for composed components --
976+
// wasmtime-environ reported a smaller number of resources, and the check was geared towards a *single* component,
977+
// not tying a particular component to a particular table (it is possible for *sub components* to originate different
978+
// resources).
979+
//
980+
// In theory, it should be posible to build this knowledge statically rather than at resource creation,
981+
// so in the future we should attempt to rebulid that code, if possible.
982+
//
983+
// Note that at runtime wasmtime maintains this information and looks it up during a transfer operation, see:
984+
// - https://github.com/bytecodealliance/wasmtime/blob/5f3b67ea055857020bd1ac1f2c3f7fa2e6c31ec0/crates/wasmtime/src/runtime/component/instance.rs#L424
985+
// - https://github.com/bytecodealliance/wasmtime/blob/9c49989a2e71382fd8639288088472f150f2f534/crates/wasmtime/src/runtime/vm/component.rs#L847
997986
}
998987

999988
/// Ensure a component-local `error-context` table has been created
@@ -1078,34 +1067,48 @@ impl<'a> Instantiator<'a, '_> {
10781067
.bindgen
10791068
.intrinsic(Intrinsic::Resource(ResourceIntrinsic::ResourceTableRemove));
10801069

1070+
// Create the relevant handle table
10811071
let rtid = resource_table_idx.as_u32();
10821072
if is_imported {
1073+
// imported
10831074
uwriteln!(
10841075
self.src.js,
1085-
"const handleTable{rtid} = [{rsc_table_flag}, 0];",
1076+
r#"
1077+
const handleTable{rtid} = [{rsc_table_flag}, 0];
1078+
handleTable{rtid}._tableID = {rtid};
1079+
handleTable{rtid}._createdReps = new Set();
1080+
"#,
10861081
);
10871082
if !self.resources_initialized.contains_key(&resource_idx) {
10881083
let ridx = resource_idx.as_u32();
10891084
uwriteln!(
10901085
self.src.js,
1091-
"const captureTable{ridx} = new Map();
1092-
let captureCnt{ridx} = 0;"
1086+
r#"
1087+
const captureTable{ridx} = new Map();
1088+
let captureCnt{ridx} = 0;
1089+
"#
10931090
);
10941091
self.resources_initialized.insert(resource_idx, true);
10951092
}
10961093
} else {
1094+
// non imported
10971095
let finalization_registry_create = self
10981096
.bindgen
10991097
.intrinsic(Intrinsic::FinalizationRegistryCreate);
11001098
uwriteln!(
11011099
self.src.js,
1102-
"const handleTable{rtid} = [{rsc_table_flag}, 0];
1103-
const finalizationRegistry{rtid} = {finalization_registry_create}((handle) => {{
1104-
const {{ rep }} = {rsc_table_remove}(handleTable{rtid}, handle);{maybe_dtor}
1105-
}});
1106-
",
1100+
r#"
1101+
const handleTable{rtid} = [{rsc_table_flag}, 0];
1102+
handleTable{rtid}._tableID = {rtid};
1103+
handleTable{rtid}._createdReps = new Set();
1104+
const finalizationRegistry{rtid} = {finalization_registry_create}((handle) => {{
1105+
const {{ rep }} = {rsc_table_remove}(handleTable{rtid}, handle);{maybe_dtor}
1106+
}});
1107+
"#,
11071108
);
11081109
}
1110+
1111+
// Add the handle table to the global list
11091112
uwriteln!(self.src.js, "{handle_tables}[{rtid}] = handleTable{rtid};");
11101113
self.resource_tables_initialized
11111114
.insert(resource_table_idx, true);

0 commit comments

Comments
 (0)