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

fix: Gracefully handle errors when disabling C modules #530

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Sculas
Copy link

@Sculas Sculas commented Feb 15, 2025

This PR makes a small change to mlua so it no longer panics when unable to disable C modules.

This is primarily meant for Pluto, which has sandbox options that fully disable loading C modules, which would otherwise conflict with mlua also trying to disable a loader that no longer exists.

I'm not 100% sure if throwing the error away is the best solution, but it seems extremely unlikely that this would fail in any logical sense besides package.loadlib or the searchers not existing, in which case the end goal was already reached (being unable to load any C modules).

I'm happy to fix this differently if you have any other ideas!

@khvzak
Copy link
Member

khvzak commented Feb 24, 2025

I'm working on adding Pluto support to next mlua and will check.
Are you using a custom Pluto build with some functionality disabled?

@Sculas
Copy link
Author

Sculas commented Feb 24, 2025

See my comment in my other PR first (#529 (comment)), compiling with the following flags:

use pluto_build as pluto;

fn main() {
    println!("cargo:rerun-if-changed=build.rs");
    pluto::Build::new()
        .opt_no_binaries()
        .compile();
}

Results in Pluto modifying the searchers table, while mlua tries to do the same (which cannot be disabled unless you use unsafe):

mlua/src/state.rs

Lines 1950 to 1958 in e706ae4

let searchers: Table = package.get("searchers")?;
#[cfg(any(feature = "lua51", feature = "luajit"))]
let searchers: Table = package.get("loaders")?;
let loader = self.create_function(|_, ()| Ok("\n\tcan't load C modules in safe mode"))?;
// The third and fourth searchers looks for a loader as a C library
searchers.raw_set(3, loader)?;
searchers.raw_remove(4)?;

Please note that Pluto does more than just this when you disable binaries, so "just don't use that option" is not a proper solution.

However, given that Pluto support is being explicitly added to mlua, you can also consider feature-gating this for Pluto.
But that includes a side effect where if someone doesn't disable binaries, mlua won't disable C modules anymore, which may also not be what you want. Hence, silently ignoring the error might be the best option here.

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

Successfully merging this pull request may close these issues.

2 participants