From 2e977a9c215d9453e421e7e5999a178fba5b34ec Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Thu, 14 Apr 2016 16:24:23 +0200 Subject: [PATCH] gc integration: __traverse__ method --- src/lib.rs | 2 +- src/py_class/gc.rs | 141 +++++++++++++++++++++++++++++++++++++++ src/py_class/members.rs | 6 +- src/py_class/mod.rs | 3 +- src/py_class/py_class.rs | 49 +++++++++++++- src/py_class/slots.rs | 21 ++++++ tests/test_class.rs | 29 +++++++- 7 files changed, 246 insertions(+), 5 deletions(-) create mode 100644 src/py_class/gc.rs diff --git a/src/lib.rs b/src/lib.rs index 3a9e7ab0..0eba36cd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -147,7 +147,7 @@ pub mod _detail { pub use ::ffi::*; } pub mod libc { - pub use ::libc::c_char; + pub use ::libc::{c_char, c_void, c_int}; } pub use err::{from_owned_ptr_or_panic, result_from_owned_ptr}; pub use function::{handle_callback, py_fn_impl, AbortOnDrop}; diff --git a/src/py_class/gc.rs b/src/py_class/gc.rs new file mode 100644 index 00000000..6df14983 --- /dev/null +++ b/src/py_class/gc.rs @@ -0,0 +1,141 @@ +// Copyright (c) 2016 Daniel Grunwald +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +use libc; +use ffi; +use std::mem; +use python::{Python, PythonObject, PyDrop, ToPythonPointer}; +use objects::PyObject; +use function::AbortOnDrop; + +// TODO: what's the semantics of the traverse return code? +// If it's just a normal python exception, we might want to use PyErr instead. +pub struct TraverseError(libc::c_int); + +#[derive(Copy, Clone)] +pub struct VisitProc<'a> { + visit: ffi::visitproc, + arg: *mut libc::c_void, + /// VisitProc contains a Python instance to ensure that + /// 1) it is cannot be moved out of the traverse() call + /// 2) it cannot be sent to other threads + py: Python<'a> +} + +impl <'a> VisitProc<'a> { + pub fn call(&self, obj: &T) -> Result<(), TraverseError> + where T: PythonObject + { + let r = unsafe { (self.visit)(obj.as_ptr(), self.arg) }; + if r == 0 { + Ok(()) + } else { + Err(TraverseError(r)) + } + } +} + +#[macro_export] +#[doc(hidden)] +macro_rules! py_class_traverse { + ($class_name:ident, + /* gc: */ { + /* traverse_proc: */ None, + /* traverse_data: */ [ ] + }) => { + // If there's nothing to traverse, we don't need to generate + // tp_traverse. + // Note that in this case, py_class_type_object_flags! must not + // use Py_TPFLAGS_HAVE_GC. + None + }; + ($class_name:ident, + /* gc: */ { + $traverse_proc: expr, + /* traverse_data: */ [] + }) => {{ + unsafe extern "C" fn tp_traverse( + slf: *mut $crate::_detail::ffi::PyObject, + visit: $crate::_detail::ffi::visitproc, + arg: *mut $crate::_detail::libc::c_void + ) -> $crate::_detail::libc::c_int + { + $crate::py_class::gc::tp_traverse::<$class_name, _>( + concat!(stringify!($class_name), ".__traverse__"), + slf, visit, arg, $traverse_proc) + } + Some(tp_traverse) + }}; +} + +#[doc(hidden)] +pub unsafe fn tp_traverse( + location: &str, + slf: *mut ffi::PyObject, + visit: ffi::visitproc, + arg: *mut libc::c_void, + callback: F +) -> libc::c_int +where C: PythonObject, + F: FnOnce(&C, Python, VisitProc) -> Result<(), TraverseError> +{ + let guard = AbortOnDrop(location); + let py = Python::assume_gil_acquired(); + let visit = VisitProc { visit: visit, arg: arg, py: py }; + let slf = PyObject::from_borrowed_ptr(py, slf).unchecked_cast_into::(); + let ret = match callback(&slf, py, visit) { + Ok(()) => 0, + Err(TraverseError(code)) => code + }; + slf.release_ref(py); + mem::forget(guard); + ret +} + +/* +/// Trait that has to be implemented by `#[gc_traverse]` members. +pub trait Traversable { + /// Call VisitProc for all python objects owned by this value. + fn traverse(&self, py: Python, visit: VisitProc) -> Result<(), TraverseError>; +} + +impl Traversable for T where T: PythonObject { + fn traverse(&self, _py: Python, visit: VisitProc) -> Result<(), TraverseError> { + visit.call(self.as_object()) + } +} + +impl Traversable for Option where T: Traversable { + fn traverse(&self, py: Python, visit: VisitProc) -> Result<(), TraverseError> { + match *self { + Some(ref val) => val.traverse(py, visit), + None => Ok(()) + } + } +} + +impl Traversable for Vec where T: Traversable { + fn traverse(&self, py: Python, visit: VisitProc) -> Result<(), TraverseError> { + for val in self { + try!(val.traverse(py, visit)); + } + Ok(()) + } +} +*/ + diff --git a/src/py_class/members.rs b/src/py_class/members.rs index 6d56a75a..d8db0881 100644 --- a/src/py_class/members.rs +++ b/src/py_class/members.rs @@ -29,6 +29,10 @@ use ffi; pub trait TypeMember where T: PythonObject { /// Convert the type member into a python object /// that can be stored in the type dict. + /// + /// Because the member may expect `self` values to be of type `T`, + /// `ty` must be T::type_object() or a derived class. + /// (otherwise the behavior is undefined) unsafe fn into_descriptor(self, py: Python, ty: *mut ffi::PyTypeObject) -> PyResult; } @@ -86,7 +90,7 @@ macro_rules! py_class_instance_method { } unsafe { let method_def = py_method_def!(stringify!($f), 0, wrap_instance_method::<()>); - $crate::py_class::members::create_instance_method_descriptor(method_def) + $crate::py_class::members::create_instance_method_descriptor::<$class>(method_def) } }} } diff --git a/src/py_class/mod.rs b/src/py_class/mod.rs index 0dfb7e37..d1520f4b 100644 --- a/src/py_class/mod.rs +++ b/src/py_class/mod.rs @@ -19,9 +19,10 @@ mod py_class; #[doc(hidden)] pub mod slots; #[doc(hidden)] pub mod members; +pub mod gc; use libc; -use std::{mem, ptr}; +use std::{mem, ptr, cell}; use python::{self, Python, PythonObject}; use objects::{PyObject, PyType}; use err::{self, PyResult}; diff --git a/src/py_class/py_class.rs b/src/py_class/py_class.rs index 5f6dfdfa..3b252a27 100644 --- a/src/py_class/py_class.rs +++ b/src/py_class/py_class.rs @@ -170,6 +170,10 @@ macro_rules! py_class { /* info: */ { /* base_type: */ $crate::PyObject, /* size: */ <$crate::PyObject as $crate::py_class::BaseObject>::size(), + /* gc: */ { + /* traverse_proc: */ None, + /* traverse_data: */ [ /*name*/ ] + }, /* data: */ [ /* { offset, name, type } */ ] // TODO: base type, documentation, ... } @@ -195,6 +199,7 @@ macro_rules! py_class_impl { /* info: */ { $base_type:ty, $size:expr, + $gc:tt, /* data: */ [ $( { $data_offset:expr, $data_name:ident, $data_ty:ty } )* ] } $slots:tt { $( $imp:item )* } $members:tt; @@ -288,7 +293,7 @@ macro_rules! py_class_impl { // hide statics in create_instance to avoid name conflicts static mut type_object : $crate::_detail::ffi::PyTypeObject - = py_class_type_object_static_init!($class, $slots); + = py_class_type_object_static_init!($class, $gc, $slots); static mut init_active: bool = false; // trait implementations that need direct access to type_object @@ -344,6 +349,7 @@ macro_rules! py_class_impl { /* info: */ { $base_type: ty, $size: expr, + $gc: tt, [ $( $data:tt )* ] } $slots:tt { $( $imp:item )* } $members:tt; @@ -353,6 +359,7 @@ macro_rules! py_class_impl { /* info: */ { $base_type, /* size: */ $crate::py_class::data_new_size::<$data_type>($size), + $gc, /* data: */ [ $($data)* { @@ -452,6 +459,46 @@ macro_rules! py_class_impl { py_error! { "__del__ is not supported by py_class!; Use a data member with a Drop impl instead." } }; + // def __traverse__(self, visit) + { $class:ident $py:ident + /* info: */ { + $base_type: ty, + $size: expr, + /* gc: */ { + /* traverse_proc: */ None, + $traverse_data: tt + }, + $datas: tt + } + $slots:tt { $( $imp:item )* } $members:tt; + def __traverse__(&$slf:tt, $visit:ident) $body:block $($tail:tt)* + } => { py_class_impl! { + $class $py + /* info: */ { + $base_type, + $size, + /* gc: */ { + /* traverse_proc: */ $class::__traverse__, + $traverse_data + }, + $datas + } + $slots + /* impl: */ { + $($imp)* + py_coerce_item!{ + impl $class { + fn __traverse__(&$slf, + $py: $crate::Python, + $visit: $crate::py_class::gc::VisitProc) + -> Result<(), $crate::py_class::gc::TraverseError> + $body + } + } + } + $members; $($tail)* + }}; + // TODO: Not yet implemented: { $class:ident $py:ident $info:tt $slots:tt $impls:tt $members:tt; def __repr__ $($tail:tt)* diff --git a/src/py_class/slots.rs b/src/py_class/slots.rs index d4275a6a..5215645b 100644 --- a/src/py_class/slots.rs +++ b/src/py_class/slots.rs @@ -24,6 +24,7 @@ use python::Python; #[doc(hidden)] macro_rules! py_class_type_object_static_init { ($class_name:ident, + $gc:tt, /* slots: */ { /* type_slots */ [ $( $slot_name:ident : $slot_value:expr, )* ] $as_number:tt @@ -32,12 +33,32 @@ macro_rules! py_class_type_object_static_init { $crate::_detail::ffi::PyTypeObject { $( $slot_name : $slot_value, )* tp_dealloc: Some($crate::py_class::slots::tp_dealloc_callback::<$class_name>), + tp_flags: py_class_type_object_flags!($gc), + tp_traverse: py_class_traverse!($class_name, $gc), .. $crate::_detail::ffi::PyTypeObject_INIT } ); } +#[macro_export] +#[doc(hidden)] +macro_rules! py_class_type_object_flags { + (/* gc: */ { + /* traverse_proc: */ None, + /* traverse_data: */ [ /*name*/ ] + }) => { + $crate::_detail::ffi::Py_TPFLAGS_DEFAULT + }; + (/* gc: */ { + $traverse_proc: expr, + $traverse_data: tt + }) => { + $crate::_detail::ffi::Py_TPFLAGS_DEFAULT + | $crate::_detail::ffi::Py_TPFLAGS_HAVE_GC + }; +} + #[macro_export] #[doc(hidden)] macro_rules! py_class_type_object_dynamic_init { diff --git a/tests/test_class.rs b/tests/test_class.rs index f3011254..10ef3a28 100644 --- a/tests/test_class.rs +++ b/tests/test_class.rs @@ -2,7 +2,8 @@ #[macro_use] extern crate cpython; -use cpython::{PyResult, Python, NoArgs, ObjectProtocol, PyDict}; +use cpython::{PyObject, PythonObject, PyDrop, PyClone, PyResult, Python, NoArgs, ObjectProtocol, PyDict}; +use std::cell::RefCell; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; @@ -234,3 +235,29 @@ fn static_data() { assert!(py.run("C.VAL1 = 124", None, Some(&d)).is_err()); } +py_class!(class GCIntegration |py| { + data self_ref: RefCell; + data dropped: TestDropCall; + + def __traverse__(&self, visit) { + visit.call(&*self.self_ref(py).borrow()) + } +}); + +#[test] +fn gc_integration() { + let gil = Python::acquire_gil(); + let py = gil.python(); + + let drop_called = Arc::new(AtomicBool::new(false)); + let inst = GCIntegration::create_instance(py, + RefCell::new(py.None()), + TestDropCall { drop_called: drop_called.clone() } + ).unwrap(); + *inst.self_ref(py).borrow_mut() = inst.as_object().clone_ref(py); + inst.release_ref(py); + + py.run("import gc; gc.collect()", None, None).unwrap(); + assert!(drop_called.load(Ordering::Relaxed)); +} +