DelSkayn / rquickjs

High level bindings to the quickjs javascript engine
MIT License
431 stars 58 forks source link

Strange behaviour of floats when JVM is present in the same memory: Floats suddenly recognized as ints #281

Open StefanRichterHuber opened 3 months ago

StefanRichterHuber commented 3 months ago

Hello, This is probably out-of-scope of your work, so please close if not appropriate. I observed a strange behavior using your library in a environment sharing memory with a Java JVM. Either as JNI library (as dynamic library loaded by the Java JVM) or the project itself loading a JVM (by loading a dynamic library) using the jni invocation feature. I am completely lost, where to search for the bug. So may be you can give me some hints, were to deepen my search, anyway.

For some reasons, floats resulting from an eval("2.34") call are recognized as int values (including the tag value of 0) and rounded to the next lower int value (in this example 2). This happens completely reproducible for all kinds of floats generated during eval calls. It does not, however, happen if I set a global value and retrieve it.

All other other types of values (JS Strings, Ints, Objects, Functions) don't show this behavior, they work fully reproducible as intended.

Environment: Ubuntu 23.10 64-Bit Java HotSpot(TM) 64-Bit Server VM Oracle GraalVM 21.0.2+13.1 rustc 1.76.0 cargo 1.76.0

Here is an example (full Gist including Cargo.toml https://gist.github.com/StefanRichterHuber/acd68ecb6571cac5df498c6b28f430ba )

#[cfg(test)]
mod tests {
    use jni::{InitArgsBuilder, JNIVersion, JavaVM};
    use rquickjs::{qjs::JSValueUnion, Context, Runtime, Value};

   /// Utility method to launch a JVM 
    fn launch_vm() -> JavaVM {
        // Build the VM properties
        let jvm_args = InitArgsBuilder::new()
            .version(JNIVersion::V8)
            .build()
            .unwrap();

        // Create a new VM
        let jvm: JavaVM = JavaVM::new(jvm_args).unwrap();
        jvm
    }

    // This tests reproducible succeeds -> both values v1 and v2 are recognized as floats with the correct value
    #[test]
    pub fn test_float_without_jvm() {
        let rt = Runtime::new().unwrap();
        let ctx = Context::full(&rt).unwrap();
        ctx.with(|ctx| {
            ctx.globals().set("v1", 5.43f64).unwrap();
            let v1: Value = ctx.eval("v1").unwrap();
            println!("{:?}", v1);
            assert!(v1.is_float());

            let v2: Value = ctx.eval("2.34").unwrap();
            println!("{:?}", v2);
            assert!(v2.is_float());
        });
    }

    // This test reproducible fails -> v1 is a float, v2 is recognized as an int
    #[test]
    pub fn test_float_with_jvm() {
        let rt = Runtime::new().unwrap();
        let ctx = Context::full(&rt).unwrap();
        let _vm = launch_vm();
        //    let mut env = vm.attach_current_thread().unwrap();
        ctx.with(|ctx| {
            ctx.globals().set("v1", 5.43f64).unwrap();
            let v1: Value = ctx.eval("v1").unwrap();  // v1.value.tag == 7 for float
            println!("{:?}", v1);
            assert!(v1.is_float()); // v1 is of type float with value 5.43

            let v2: Value = ctx.eval("2.34").unwrap();  // v2.value.tag == 0 for int. v2.value.int32 == 2
            println!("{:?}", v2);
            assert!(v2.is_float()); // This fails ->  v2 is of type int with value 2
        });
    }
}
StefanRichterHuber commented 3 months ago

Since I am working on a German system, this problem is related to https://github.com/bellard/quickjs/issues/106 . Parsing of floats in QuickJS depends on the locale set .sigh As workaround I use the uselocale function from the libccrate to temporarily modify the locale in the current thread before executing eval.

DelSkayn commented 2 months ago

Hmm, this is a problem, setting the locale temporarily will cause problems when used in a multi-threaded environment. Upstream seems to not be interested in supporting other locales.

Maybe we could somehow patch quickjs to bind atof to a rust implemented version using rust std string parsing? Will look into it.