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

PyVisit::call segfaults when passed None #2915

Closed
neachdainn opened this issue Jan 26, 2023 · 2 comments · Fixed by #2921
Closed

PyVisit::call segfaults when passed None #2915

neachdainn opened this issue Jan 26, 2023 · 2 comments · Fixed by #2921
Labels

Comments

@neachdainn
Copy link
Contributor

Bug Description

When PyVisit::call is passed a None value, this is converted into a null pointer and passed to the underlying visitproc function. However, (by my understanding) the visitproc function isn't meant to handle null pointers as the intended interface is actually Py_VISIT which does do a null check.

Steps to Reproduce

Minimal example:

use pyo3::{prelude::*, PyVisit, PyTraverseError, types::PyType};

#[pyclass]
struct Foo {
    #[pyo3(get, set)]
    other: Option<Py<Foo>>,

    obj: Option<Py<Bar>>,
}

#[pymethods]
impl Foo {
    #[new]
    fn new() -> Self {
        Self { other: None, obj: None }
    }

    fn __traverse__(&self, visit: PyVisit<'_>)  -> Result<(), PyTraverseError> {
        // Segfaults
        visit.call(&self.obj)?;
        Ok(())
    }

    fn __clear__(&mut self) {
        self.obj = None;
    }
}

#[pyclass]
struct Bar;

fn main()
{
    pyo3::prepare_freethreaded_python();
    Python::with_gil(|py| {
        let foo = PyType::new::<Foo>(py);
        pyo3::py_run!(py, foo foo, r#"
            import gc
            import time

            item = foo()
            item.other = item
            del item

            gc.collect()
        "#);
    });
}

Backtrace

No response

Your operating system and version

Ubuntu 22, Ubuntu 20

Your Python version (python --version)

Python 3.10.6, Python 3.8.10

Your Rust version (rustc --version)

rustc 1.66.1

Your PyO3 version

PyO3 0.17.0, PyO3 0.18.0

How did you install python? Did you use a virtualenv?

System installation

Additional Info

No response

@neachdainn neachdainn added the bug label Jan 26, 2023
@neachdainn
Copy link
Contributor Author

I will open a PR tomorrow. I just wanted to document this and open it up for conversation before I left work for the day.

@davidhewitt
Copy link
Member

Thanks for reporting, yep agreed this is a bug, PR much appreciated!

neachdainn added a commit to neachdainn/pyo3 that referenced this issue Jan 26, 2023
Closes PyO3#2915

When using the C API directly, the intended way to call `visitproc` is
via the `Py_VISIT` macro, which checks to see that the provided pointer
is not null before passing it along to `visitproc`. Because PyO3 isn't
using the macro, it needs to manually check that the pointer isn't null.
Without this check, calling `visit.call(&obj)` where `let obj = None;`
will segfault.
neachdainn added a commit to neachdainn/pyo3 that referenced this issue Jan 26, 2023
Closes PyO3#2915

When using the C API directly, the intended way to call `visitproc` is
via the `Py_VISIT` macro, which checks to see that the provided pointer
is not null before passing it along to `visitproc`. Because PyO3 isn't
using the macro, it needs to manually check that the pointer isn't null.
Without this check, calling `visit.call(&obj)` where `let obj = None;`
will segfault.
bors bot added a commit that referenced this issue Jan 27, 2023
2904: refactor docstring generation code r=mejrs a=davidhewitt

As a first step towards #2866 this is a tweak to the macro code which generates Python docstrings. This PR refactors the behaviour so that instead of always creating a `concat!` expression to generate a nul-terminated string, this will only happen if a Rust 1.54+ macro doc is present (e.g. `#[doc = include_str!(...)]`).

The default case now just collects all the `#[doc]` attributes into a single string.

This should make it easier to factor out the `text_signature` formatting, and avoids wasting compile time invoking the `concat!` macro when not necessary.

2921: Check to see if object is `None` before traversing r=davidhewitt a=neachdainn

Closes #2915

When using the C API directly, the intended way to call `visitproc` is via the `Py_VISIT` macro, which checks to see that the provided pointer is not null before passing it along to `visitproc`. Because PyO3 isn't using the macro, it needs to manually check that the pointer isn't null. Without this check, calling `visit.call(&obj)` where `let obj = None;` will segfault.


Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
Co-authored-by: Nate Kent <nate@nkent.net>
@bors bors bot closed this as completed in f38841a Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants