Skip to content
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: abstraction of ffi api libraries loading #1968

Open
lowlighter opened this issue Feb 26, 2022 · 7 comments
Open

feat: abstraction of ffi api libraries loading #1968

lowlighter opened this issue Feb 26, 2022 · 7 comments

Comments

@lowlighter
Copy link
Contributor

Is your feature request related to a problem? Please describe.

The FFI API starting boilerplate feels like it could be abstracted in deno std lib to be more friendly.

I feel like the switch should be put under the carpet and users should just pass the lib without the extension and the underlying function automatically append the correct one. I know that extensions are conventions, but users who don't wish to use the most commons one could still fallback on the current solution.

Currently you need the following:

// Determine library extension based on
// your OS.
let libSuffix = "";
switch (Deno.build.os) {
  case "windows":
    libSuffix = "dll";
    break;
  case "darwin":
    libSuffix = "dylib";
    break;
  case "linux":
    libSuffix = "so";
    break;
}

const libName = `./libadd.${libSuffix}`;
// Open library and define exported symbols
const dylib = Deno.dlopen(libName, {
  "add": { parameters: ["isize", "isize"], result: "isize" },
});

Granted, it's a small incovenience of ~10 lines of code (which could be even less) but it'd be nice anyway.

Describe the solution you'd like

Something along:

import { load } from "https://deno.land/std/ffi/load.ts"
const dylib = load("./libadd", {
    "add": { parameters: ["isize", "isize"], result: "isize" },
});

Describe alternatives you've considered

Currently using the first solution

@littledivy
Copy link
Member

On Windows, dynamic libraries generally don't have lib prefix. So it's add.dll.

Also, This is useful but its not enough for most libraries that need to release libraries and cache them on first run. Currently, deno_bindgen use x/plug for that - maybe we could have something similar to x/plug in deno_std.

Note that x/plug is not complete too, it lacks support for loading aarch64-darwin libraries.

@lino-levan
Copy link
Contributor

For anyone reading this thread in the future, x/plug does now support loading aarch64-darwin libraries since version 0.5.2

As for this specific issue, I don't really see this as something std should do, given that x/plug does already exist. Would love to get an opinion from someone on the core team though.

@iuioiua
Copy link
Collaborator

iuioiua commented Nov 23, 2023

For anyone reading this thread in the future, x/plug does now support loading aarch64-darwin libraries since version 0.5.2

As for this specific issue, I don't really see this as something std should do, given that x/plug does already exist. Would love to get an opinion from someone on the core team though.

I'm happy to close this issue then if that's the case. WDYT, @kt3k?

@kt3k
Copy link
Member

kt3k commented Nov 23, 2023

I'd like to hear the opinion from @aapoalas

@lino-levan
Copy link
Contributor

Definitely would be good to hear some thoughts from Aapo. I should note that /x/plug hit 1.0 and is considered stable by the maintainers afaik.

@aapoalas
Copy link
Collaborator

aapoalas commented Dec 4, 2023

My apologies, I had somehow missed the ping here.

I do concur with the above comments in the abstract: plug exists and it doesn't seem like a particularly necessary thing to duplicate it.

On the other hand, I've come to the conclusion that I myself am not comfortable running any 3rd party code alongside FFI permissions and thus any and all FFI related "required" convenience code can either only be locally duplicated or used from deno_std if it exists there. In this sense implementing this helper in std would make sense.

But, I am currently trying to pursue FFI API stabilisation and one of the things being looked at within that is loading of libraries. It might be that it becomes more interconnected with the module graph in which case this helper would need to change.

Another thing is my proposal/PR for FFI tokens which would enable FFI permission path gating among other things. That might make FFI restricted enough to reasonably allow running 3rd party code that does DLL loading like plug. Though plug is really quite dangerous (again in the abstract): It is trusted to download, save, and load a DLL for the user. A malicious actor could do this and load a secondary library alongside the first that is used to do whatever it wants.

From a security perspective it is a good idea to consider this in std. I'd say let's leave this open for now and look back at it once the FFI stabilisation moves forwards (or backwards).

@kt3k
Copy link
Member

kt3k commented Dec 4, 2023

Note: @aapoalas's proposal is discussed here: denoland/deno#19099

I generally agree with the above. Let's keep this open and revisit when stabilization or token proposal made any progress

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants