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

Potential memory leak when calling Python from Rust #1801

Closed
pschafhalter opened this issue Aug 16, 2021 · 7 comments · Fixed by #1806 or #1810
Closed

Potential memory leak when calling Python from Rust #1801

pschafhalter opened this issue Aug 16, 2021 · 7 comments · Fixed by #1806 or #1810

Comments

@pschafhalter
Copy link
Contributor

🐛 Bug Reports

Repeatedly calling Python from Rust appears to cause a memory leak of ~700 bytes per invocation. This poses a problem for long-lived Rust programs that repeatedly call Python. In my case, I ran into this issue while adding support for Python UDFs to a Rust-based streaming system.

As a side note, thanks for all the hard work on this fantastic library! Hoping this can be resolved, and that I haven't missed something on my end.

🌍 Environment

  • Your operating system and version: Ubuntu 20.04 LTS
  • Your python version: 3.8.10
  • How did you install python (e.g. apt or pyenv)? Did you use a virtualenv?: Installed via apt, using a virtualenv.
  • Your Rust version (rustc --version): 1.54.0
  • Your PyO3 version: 0.14.2
  • Have you tried using latest PyO3 main (replace version = "0.x.y" with git = "https://github.com/PyO3/pyo3")?: yes

💥 Reproducing

use pyo3::prelude::*;

fn main() {
    loop {
        Python::with_gil(move |py| {
            py.run("1 + 1", None, None).unwrap();
        }); 
    }
}

The following also causes the issue:

use pyo3::prelude::*;

fn main() {
    Python::with_gil(move |py| {
        loop {
            py.run("1 + 1", None, None).unwrap();
        }
    }); 
}

Run with cargo run --release and observe memory use grow.

@pschafhalter
Copy link
Contributor Author

I ran valgrind according to the documentation on the following program to check for leaks:

use pyo3::prelude::*;

fn main() {
    Python::with_gil(move |py| {
        for _ in 0..1_000 {
            py.run("1 + 1", None, None).unwrap();
        }
    }); 
}

Output:

==440563== Memcheck, a memory error detector
==440563== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==440563== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==440563== Command: ./target/debug/pyo3_mem_leak --with-options
==440563== 
==440563== 
==440563== HEAP SUMMARY:
==440563==     in use at exit: 600,248 bytes in 394 blocks
==440563==   total heap usage: 7,752 allocs, 7,358 frees, 54,156,267 bytes allocated
==440563== 
==440563== LEAK SUMMARY:
==440563==    definitely lost: 0 bytes in 0 blocks
==440563==    indirectly lost: 0 bytes in 0 blocks
==440563==      possibly lost: 600,248 bytes in 394 blocks
==440563==    still reachable: 0 bytes in 0 blocks
==440563==         suppressed: 0 bytes in 0 blocks
==440563== Rerun with --leak-check=full to see details of leaked memory
==440563== 
==440563== For lists of detected and suppressed errors, rerun with: -s
==440563== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2968 from 6)

I've also attached a logfile for running valgrind with --leak-check=full:
valgrind_leak_check_full.log

@mejrs
Copy link
Member

mejrs commented Aug 17, 2021

Thank you :). I think you're right.

Py_CompileString returns an owned pointer, I think this should be decref'd? From a quick test that stops the memory leak.

https://github.com/PyO3/pyo3/blob/main/src/python.rs#L394

Note however, that your second example (with a loop inside the with_gil closure) will still "leak memory" because you never release the GIL. If necessary you can use Python::new_pool to do so.

@pschafhalter
Copy link
Contributor Author

Thanks for the tip, adding ffi::Py_DECREF(code_obj); to the run_code function fixes the issue :)

If it's any help, I'm happy to make a PR. Otherwise, I can just run pyo3 with the patch for now.

@messense
Copy link
Member

If it's any help, I'm happy to make a PR. Otherwise, I can just run pyo3 with the patch for now.

Please do! Pull requests are always welcome.

@Ptrskay3
Copy link
Contributor

I checked that Py_CompileString is also used here, and seemigly it's also not Py_DECREF'd. I'm fairly new to PyO3 and slowly discovering the codebase, but I'm glad to fix this, if needed.

@birkenfeld
Copy link
Member

Good find, I agree that needs a DECREF too.

@birkenfeld
Copy link
Member

Reopening for the second instance.

@birkenfeld birkenfeld reopened this Aug 18, 2021
haixuanTao added a commit to dora-rs/dora that referenced this issue Jan 3, 2023
This commit fix the memory leak happening in the dora API operator. This
is seemingly due to pyo3 leaking memory on object created in Rust.
Using standard `drop` did not drop the memory on the `PyBytes` included in
the `PyDict`.

See: PyO3/pyo3#1801

Fixes #163
haixuanTao added a commit to dora-rs/dora that referenced this issue Jan 14, 2023
This commit fix the memory leak happening in the dora API operator. This
is seemingly due to pyo3 leaking memory on object created in Rust.
Using standard `drop` did not drop the memory on the `PyBytes` included in
the `PyDict`.

See: PyO3/pyo3#1801

Fixes #163
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants