Skip to content

Commit

Permalink
feat(ext/ffi): support marking symbols as optional (denoland#18529)
Browse files Browse the repository at this point in the history
  • Loading branch information
DjDeveloperr committed Apr 3, 2023
1 parent 51d3fb7 commit 62c5664
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 6 deletions.
14 changes: 13 additions & 1 deletion cli/tsc/dts/lib.deno.unstable.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,11 @@ declare namespace Deno {
*
* @default {false} */
callback?: boolean;
/** When `true`, dlopen will not fail if the symbol is not found.
* Instead, the symbol will be set to `null`.
*
* @default {false} */
optional?: boolean;
}

/** **UNSTABLE**: New API, yet to be vetted.
Expand All @@ -282,6 +287,11 @@ declare namespace Deno {
name?: string;
/** The type of the foreign static value. */
type: Type;
/** When `true`, dlopen will not fail if the symbol is not found.
* Instead, the symbol will be set to `null`.
*
* @default {false} */
optional?: boolean;
}

/** **UNSTABLE**: New API, yet to be vetted.
Expand Down Expand Up @@ -336,7 +346,9 @@ declare namespace Deno {
* @category FFI
*/
type StaticForeignLibraryInterface<T extends ForeignLibraryInterface> = {
[K in keyof T]: StaticForeignSymbol<T[K]>;
[K in keyof T]: T[K]["optional"] extends true
? StaticForeignSymbol<T[K]> | null
: StaticForeignSymbol<T[K]>;
};

const brand: unique symbol;
Expand Down
7 changes: 7 additions & 0 deletions ext/ffi/00_ffi.js
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,12 @@ class DynamicLibrary {
continue;
}

// Symbol was marked as optional, and not found.
// In that case, we set its value to null in Rust-side.
if (symbols[symbol] === null) {
continue;
}

if (ReflectHas(symbols[symbol], "type")) {
const type = symbols[symbol].type;
if (type === "void") {
Expand All @@ -456,6 +462,7 @@ class DynamicLibrary {
this.#rid,
name,
type,
symbols[symbol].optional,
);
ObjectDefineProperty(
this.symbols,
Expand Down
23 changes: 19 additions & 4 deletions ext/ffi/dlfcn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,19 @@ pub struct ForeignFunction {
#[serde(rename = "callback")]
#[serde(default = "default_callback")]
callback: bool,
#[serde(rename = "optional")]
#[serde(default = "default_optional")]
optional: bool,
}

fn default_callback() -> bool {
false
}

fn default_optional() -> bool {
false
}

// ForeignStatic's name and type fields are read and used by
// serde_v8 to determine which variant a ForeignSymbol is.
// They are not used beyond that and are thus marked with underscores.
Expand Down Expand Up @@ -156,7 +163,7 @@ where
ForeignSymbol::ForeignStatic(_) => {
// No-op: Statics will be handled separately and are not part of the Rust-side resource.
}
ForeignSymbol::ForeignFunction(foreign_fn) => {
ForeignSymbol::ForeignFunction(foreign_fn) => 'register_symbol: {
let symbol = match &foreign_fn.name {
Some(symbol) => symbol,
None => &symbol_key,
Expand All @@ -168,10 +175,18 @@ where
// SAFETY: The obtained T symbol is the size of a pointer.
match unsafe { resource.lib.symbol::<*const c_void>(symbol) } {
Ok(value) => Ok(value),
Err(err) => Err(generic_error(format!(
"Failed to register symbol {symbol}: {err}"
))),
Err(err) => if foreign_fn.optional {
let null: v8::Local<v8::Value> = v8::null(scope).into();
let func_key = v8::String::new(scope, &symbol_key).unwrap();
obj.set(scope, func_key.into(), null);
break 'register_symbol;
} else {
Err(generic_error(format!(
"Failed to register symbol {symbol}: {err}"
)))
},
}?;

let ptr = libffi::middle::CodePtr::from_ptr(fn_ptr as _);
let cif = libffi::middle::Cif::new(
foreign_fn
Expand Down
13 changes: 12 additions & 1 deletion ext/ffi/static.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,21 @@ pub fn op_ffi_get_static<'scope>(
rid: ResourceId,
name: String,
static_type: NativeType,
optional: bool,
) -> Result<serde_v8::Value<'scope>, AnyError> {
let resource = state.resource_table.get::<DynamicLibraryResource>(rid)?;

let data_ptr = resource.get_static(name)?;
let data_ptr = match resource.get_static(name) {
Ok(data_ptr) => Ok(data_ptr),
Err(err) => {
if optional {
let null: v8::Local<v8::Value> = v8::null(scope).into();
return Ok(null.into());
} else {
Err(err)
}
}
}?;

Ok(match static_type {
NativeType::Void => {
Expand Down
15 changes: 15 additions & 0 deletions test_ffi/tests/ffi_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ const remote = Deno.dlopen(
parameters: ["bool"],
result: "bool",
},
method25: {
parameters: [],
result: "void",
optional: true,
},
static1: { type: "usize" },
static2: { type: "pointer" },
static3: { type: "usize" },
Expand All @@ -61,6 +66,10 @@ const remote = Deno.dlopen(
static13: { type: "f32" },
static14: { type: "f64" },
static15: { type: "bool" },
static16: {
type: "bool",
optional: true,
},
},
);

Expand Down Expand Up @@ -277,6 +286,9 @@ let r24_0: true = remote.symbols.method24(true);
let r42_1: number = remote.symbols.method24(true);
<boolean> remote.symbols.method24(Math.random() > 0.5);

// @ts-expect-error: Optional symbol; can be null.
remote.symbols.method25();

// @ts-expect-error: Invalid member type
const static1_wrong: null = remote.symbols.static1;
const static1_right: number | bigint = remote.symbols.static1;
Expand Down Expand Up @@ -322,6 +334,9 @@ const static14_right: number = remote.symbols.static14;
// @ts-expect-error: Invalid member type
const static15_wrong: number = remote.symbols.static15;
const static15_right: boolean = remote.symbols.static15;
// @ts-expect-error: Invalid member type
const static16_wrong: boolean = remote.symbols.static16;
const static16_right: boolean | null = remote.symbols.static16;

// Adapted from https://stackoverflow.com/a/53808212/10873797
type Equal<T, U> = (<G>() => G extends T ? 1 : 2) extends
Expand Down
17 changes: 17 additions & 0 deletions test_ffi/tests/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ const dylib = Deno.dlopen(libPath, {
*/
"static_char": {
type: "pointer",
optional: true,
},
"hash": { parameters: ["buffer", "u32"], result: "u32" },
make_rect: {
Expand Down Expand Up @@ -281,6 +282,22 @@ const dylib = Deno.dlopen(libPath, {
print_mixed: {
parameters: [{ struct: Mixed }],
result: "void",
optional: true,
},
non_existent_symbol: {
parameters: [],
result: "void",
optional: true,
},
non_existent_nonblocking_symbol: {
parameters: [],
result: "void",
nonblocking: true,
optional: true,
},
non_existent_static: {
type: "u32",
optional: true,
},
});
const { symbols } = dylib;
Expand Down

0 comments on commit 62c5664

Please sign in to comment.