pyo3
pyo3 copied to clipboard
Raising warning inside `__traverse__` causes a segfault
Code:
use pyo3::prelude::*;
use pyo3::py_run;
use pyo3::{PyTraverseError, PyVisit};
#[pyclass]
struct GcIntegration {
self_ref: PyObject,
}
#[pymethods]
impl GcIntegration {
fn __traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> {
Python::with_gil(|py| {
let warn = py.import("warnings").unwrap().getattr("warn").unwrap();
warn.call1(("blah",)).unwrap();
});
visit.call(&self.self_ref)
}
fn __clear__(&mut self) {
let gil = Python::acquire_gil();
self.self_ref = gil.python().None();
}
}
fn main() {
Python::with_gil(|py| {
let inst = PyCell::new(
py,
GcIntegration {
self_ref: py.None(),
},
)
.unwrap();
let mut borrow = inst.borrow_mut();
borrow.self_ref = inst.to_object(py);
py_run!(py, inst, "import gc; assert inst in gc.get_objects()");
});
Python::with_gil(|py| {
py.run("import gc; gc.collect()", None, None).unwrap();
});
}
Results in:
error: process didn't exit successfully: `target\debug\my_module.exe` (exit code: 0xc0000005, STATUS_ACCESS_VIOLATION)
Meta:
rustc 1.62.0-nightly (1f7fb6413 2022-04-10)
binary: rustc
commit-hash: 1f7fb6413d6d6c0c929b223e478e44c3db991b03
commit-date: 2022-04-10
host: x86_64-pc-windows-msvc
release: 1.62.0-nightly
LLVM version: 14.0.0
Python 3.10.2 (tags/v3.10.2:a58ebcc, Jan 17 2022, 14:12:15) [MSC v.1929 64 bit (AMD64)]
Yeah, ouch 🤕
I'm not sure there's anything we can do about it other than carefully document that __traverse__
is very limited? I think it's not really intended for any Python APIs to be called in it.
Should be unsafe to implement then.
While I don't disagree, there isn't a syntax for this. unsafe impl
is only for traits (so doesn't apply to #[pymethods]
), and unsafe fn
is for unsafe-to-call.
Requiring unsafe fn traverse
(even if it's not called from user code) is probably good enough.
Do we know why this segfaults? I can imagine why, but I glanced through the capi documentation and didn't see it called out anywhere.
While I don't disagree, there isn't a syntax for this.
unsafe impl
is only for traits
You can have
#[pymethods]
unsafe impl Foo {
// ...
}
This is syntactically correct but not semantically, so if the unsafe keyword is removed by the macro it will work.
I'd like something better than slapping unsafe on it and calling it a day.
What about making Python::with_gil
and Python::acquire_gil
panic (more likely: abort) if called from within __traverse__
? We can temporarily store usize::MAX
inside the GIL_COUNT
thread_local, and make those two functions check for it.
Alternatively, stop modeling __traverse__
as a freely implementable method, and make it a macro with declarative syntax, where you just give the self
members that should be visited. (Same would probably apply for __clear__
.)
Looking at CPython sources, this should cover 90% of __traverse__
implementations, and the others can be done via an unsafe
escape hatch.
I was considering similar; I once experimented with a derive macro that would cover all members automatically, but it needed specialization despite my best efforts to hack 😆
A declarative macro gets a +1 from me.
I once experimented with a derive macro
Could something like this work? With the __traverse__
impl above as an unsafe escape hatch?
#[pyclass]
struct GcIntegration {
#[pyo3(gc)]
self_ref: PyObject,
}
Potentially; I think we'd probably be able to do something like have an unsafe trait PyGcTraversal
, which we can then provide a blanket implementation for T: AsPyPointer
as well as containers like Vec<T>
.
It'll get a bit messy things like HashMap<K, V>
where depending on K
and V
we'd need to figure out how the trait implementation varies depending on whether K
and V
implement PyGcTraversal
. If I recall correctly that's where my previous hackery got stuck. At least with the approach of annotating all fields we could have #[pyo3(gc = "some_function")]
to manually supply an escape hatch to traverse complex containers?