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

Possible memory leak #800

Open
lynn2910 opened this issue Dec 29, 2023 · 4 comments
Open

Possible memory leak #800

lynn2910 opened this issue Dec 29, 2023 · 4 comments

Comments

@lynn2910
Copy link

Hello, I'm interested in experimenting with Rhai for a potential IT project and have been exploring its scope and the ability to run multiple scripts simultaneously. In my efforts to ensure efficient memory usage, I decided to run valgrind and discovered a potential memory leak. While I'm not an expert, I want to make sure this issue is thoroughly investigated to ensure its significance or lack thereof.

The code executed:

use std::collections::HashMap;
use std::ops::Deref;
use std::sync::Arc;
use rhai::{AST, Engine, Scope};
use tokio::sync::RwLock;

fn init_store() -> HashMap<String, Arc<RwLock<AST>>> {
    let global_engine = Engine::new();
    let mut ast_store = HashMap::new();
    ast_store.insert(
        "test".to_string(),
        Arc::new(RwLock::new(global_engine.compile("x + 2").unwrap()))
    );

    ast_store
}

fn create_scope<'a>() -> Scope<'a> {
    let mut scope = Scope::new();
    scope.push("x", 8i64);
    scope.push("y", 8i64);

    scope
}

#[tokio::main]
async fn main() {
    let scope = create_scope();

    let scripts = Arc::new(RwLock::new(init_store()));

    let mut handles = Vec::new();
    for i in 0..10 {
        let mut scope = scope.clone();

        let scripts_store_clone = scripts.clone();
        let handle = tokio::spawn(async move {
            let engine = Engine::new();

            let store = scripts_store_clone.read().await;
            let script = store.get("test").unwrap();

            match engine.run_ast_with_scope(&mut scope, script.read().await.deref()) {
                Ok(_) => println!("success for {i}"),
                Err(e) => println!("error for {i}: {e:#?}")
            };
        });
        handles.push(handle);
    }

    for h in handles {
        h.await.unwrap();
    }

    drop(scope);
    drop(scripts);
}

Which valgrind tell me this:

==50273== Memcheck, a memory error detector
==50273== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==50273== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==50273== Command: ../../target/release/rhai_test
==50273== 
success for 0
success for 7
success for 2
success for 1
success for 3
success for 4
success for 5
success for 6
success for 8
success for 9
==50273== 
==50273== HEAP SUMMARY:
==50273==     in use at exit: 64 bytes in 1 blocks
==50273==   total heap usage: 27,487 allocs, 27,486 frees, 2,451,100 bytes allocated
==50273== 
==50273== LEAK SUMMARY:
==50273==    definitely lost: 0 bytes in 0 blocks
==50273==    indirectly lost: 0 bytes in 0 blocks
==50273==      possibly lost: 0 bytes in 0 blocks
==50273==    still reachable: 64 bytes in 1 blocks
==50273==         suppressed: 0 bytes in 0 blocks
==50273== Rerun with --leak-check=full to see details of leaked memory
==50273== 
==50273== For lists of detected and suppressed errors, rerun with: -s
==50273== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

There is also the same problem when i run it without threads:

use std::collections::HashMap;
use std::ops::Deref;
use std::sync::Arc;
use rhai::{AST, Engine, Scope};
use tokio::sync::RwLock;

fn init_store() -> HashMap<String, Arc<RwLock<AST>>> {
    let global_engine = Engine::new();
    let mut ast_store = HashMap::new();
    ast_store.insert(
        "test".to_string(),
        Arc::new(RwLock::new(global_engine.compile("x + 2").unwrap()))
    );

    ast_store
}

fn create_scope<'a>() -> Scope<'a> {
    let mut scope = Scope::new();
    scope.push("x", 8i64);
    scope.push("y", 8i64);

    scope
}

#[tokio::main]
async fn main() {
    let mut scope = create_scope();
    let scripts = Arc::new(RwLock::new(init_store()));

    let engine = Engine::new();

    let script = scripts.get("test").unwrap();

    match engine.run_ast_with_scope(&mut scope, script.read().await.deref()) {
        Ok(_) => println!("success"),
        Err(e) => println!("error: {e:#?}")
    };

    drop(scope);
    drop(scripts);
}

The output of valgrind without threads:

==49753== Memcheck, a memory error detector
==49753== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==49753== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==49753== Command: ../../target/release/rhai_test
==49753== 
success for 0
==49753== 
==49753== HEAP SUMMARY:
==49753==     in use at exit: 64 bytes in 1 blocks
==49753==   total heap usage: 5,296 allocs, 5,295 frees, 509,860 bytes allocated
==49753== 
==49753== LEAK SUMMARY:
==49753==    definitely lost: 0 bytes in 0 blocks
==49753==    indirectly lost: 0 bytes in 0 blocks
==49753==      possibly lost: 0 bytes in 0 blocks
==49753==    still reachable: 64 bytes in 1 blocks
==49753==         suppressed: 0 bytes in 0 blocks
==49753== Rerun with --leak-check=full to see details of leaked memory
==49753== 
==49753== For lists of detected and suppressed errors, rerun with: -s
==49753== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

The Cargo.toml:

[package]
name = "rhai_test"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
rhai = { version = "1.16.3", features = ['std', 'sync']}
tokio = { version = "1.35.1", features = ["sync", "default", "macros", "rt", "rt-multi-thread"] }

I am using rustc 1.75.0 with cargo 1.75.0 on stable channel.

@schungx
Copy link
Collaborator

schungx commented Dec 29, 2023

Any idea what those 64 bytes are?

It could be the static key for function hashing, which is static data.

This may be valid engine infrastructure data that should stay on for the life of the program.

Google's OSS-Fuzz doesn't find such leaks in normal scripts (it does find leaks in quite convoluted cases where you explicitly create a memory loop).

@silvergasp
Copy link
Contributor

@schungx
Copy link
Collaborator

schungx commented Jan 12, 2024

I don't think so

oss-fuzz found memory leaks due to deliberate actions on the script to create memory loops. The leakage is much larger than this.

This is only 8 words worth of leakage and very consistent. I suspect it has something to do with once_cell which creates static objects.

@schungx
Copy link
Collaborator

schungx commented Jan 15, 2024

Hashing keys take 4 u64.

Indexer hashes take 2 u64.

So that's 6... The other two could be OnceCell overhead...

pub(crate) struct OnceCell<T> {
    queue: AtomicPtr<Waiter>,    // <-- This is one word
    value: UnsafeCell<Option<T>>,    // <-- The `Option` potentially adds one word...
}

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

3 participants