-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: ffi to replace plugins #11152
feat: ffi to replace plugins #11152
Conversation
There's already #10908 which will remove native plugins, so I suggest to skip this part in your PR - it will be easier to review. |
Sure, do so it was just to make development a bit easier (not having to deal with the old plugin system permissions mainly). |
Weird that it doesn't build on ubuntu, looks to be related to tov/libffi-rs#20 but on ubuntu (and WSL when I try it out locally) somehow? 🤔 |
The error is this:
Meaning it's failing because autoconf isn't installed on the buildbot (autoreconf is part of autoconf.) You probably need to slot that in |
Excellent, thanks for getting this going @eliassjogreen. This is a bit low level compared to what I'd like to see, it's nearly there but passing type on each invocation seems like a very low level interface to give the user. I had imagined it to be more of a top level object one would instantiate once with a descriptor, for example: const library = new ForeignFunctionInterface.Library("lib.so", {
symbols: {
foo: { parameters: ["i32"], results: ["i32"]},
bar: { parameters: ["i32", "i32"], results: ["i32"]},
},
});
library.symbols.foo(1, 2); For reference, the WebAssembly js api define an interface for typed functions: Would be nice to keep close to this API where possible. |
@eliassjogreen With this FFI would you be able to implement Deno webview programs? |
How would an async call look like? |
I'll look into it but async ffi return types will probably not be supported as standard dynamic libraries dont have any future or async type. Implementering webview should be more than doable and theoretically shouldn't even require any connecting rust or c code instead relying on the webview binaries provided by the main project. |
How about a special FFI Deno type that allows to return an async future? Meaning FFI would take care of doing the machinery for waiting for the future to be resolved, but in the C function you get an opaque type that you can resolve with a FFI Value later. |
extensions/ffi/lib.rs
Outdated
dlcall_args.args.into_iter().map(|arg| arg.into()).collect(); | ||
|
||
Ok(match dlcall_args.return_type { | ||
FFIType::Void => json!(unsafe { cif.call::<()>(fn_code_ptr, &args) }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is serde_v8
a better option here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the json!
macro automatically makes it into a serde_v8
serializable Value
if I'm not mistaken
Yesterday I mentioned in Discord that it didn't seem right that the JS caller had to specify the function signatures – they should be specified in the library instead. (The My idea was to have some standard function that all Deno ffi libraries are expected to have, and that returns a list with every exported function and their signatures. Maybe something like this: // Returns a 'static null-terminated list of 'static pointers.
export "C" fn deno_ffi_exports() -> *const [*const DenoFfiExport];
#[repr(C)]
struct DenoFfiExport {
// 'static, null-terminated, UTF-8. The unmangled name of the function.
name: *const char,
args_num: usize,
args: *const DenoFfiType,
return_type: DenoFfiType
}
#[repr(int)]
enum DenoFfiType {
Void => 0,
U8,
I8,
U16,
// ...
} For a Rust ffi library, this description could then be auto-generated by a Rust crate, similar to what wasm-bindgen does. And we'd want to provide something similar for C(++) libraries. |
I don't believe this is such a good idea as it would not allow loading of dynamic libraries not intended for deno without recompiling. With the current approach of defining the types at runtime in deno (or at dlopen) we could load most dynamic libraries. Also doing it like this is also very similar to the old plugin system and not very compiler agnostic (how would someone do it from c, c++, zig, objc or whatever other language which compiles to a dynamic library). |
Supporting callback functions in the ffi would probably be easier and more language agnostic. The libffi crate already supports passing function closures to ffi calls, with this i could make it possible to pass js functions to ffi calls using an async op which resolves when it's time to call the closure. A less language agnostic option would be a crate like async-ffi which makea futures ffi compatible. |
FYI: how to make CI compile pass, please refer to my previous PR: About FFI https://github.com/denoland/deno/pull/9173/files#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR140-R157 |
I'm very interested in this work, but unfortunately I didn't get any feedback on the PR I submitted before, until bartlomieju replied that it was being discussed internally😞 |
Do you think that the |
That sounds good, let's try to get strings/raw buffers working for 1.13 as well. |
IMHO this should be part of the first release also. |
* | ||
* Opens a dynamic library and registers symbols | ||
*/ | ||
export function dlopen<S extends Record<string, ForeignFunction>>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ffiopen
has a nice ring
@eliassjogreen I guess passing strings/buffers to ffi library might be easy but what if you do native plugin in rust and want to return a string - how would that work? I mean you have to free that string eventually and it has to be done from the native plugin again, right? for example, let's say you have webview, you load the page and then you want to evaluate some JS code there and get the result (JSON stringified) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀 this will open a lot of possibilities for integration with other languages
Thank you very much @eliassjogreen for bringing this home and all other parties involved. Let's add support for passing strings in a follow up PR as this one is already huge.
FYI if you return It does exactly the same thing if you input it to a
Maybe this is expected behavior but it should be at least documented and I would say it's not terribly useful then for native plugins. Maybe it should return |
@cztomsik can you please open an issue with this report? |
@bartlomieju sure #12212 |
This pr implements dynamic library ffi (
Deno.dlopen
andDeno.dlcall
), removes the old plugins, an example and renames theallow-plugin
permission toallow-ffi
. There is still some things to be done, mainly support for moreFFIType
s like structs, tuples and variadics. Currently the ffi only supports calling foreign functions but some things such as viewing the value of pointers and static exported variables/symbols are not supported due to concerns with both security but mainly the whole thing segfaulting.cc @bartlomieju