Patchwork [3,of,4] rust-cpython: pair leaked reference with its manager object

login
register
mail settings
Submitter Yuya Nishihara
Date Sept. 1, 2019, 1:40 p.m.
Message ID <a69f68569113eaa65e3a.1567345233@mimosa>
Download mbox | patch
Permalink /patch/41461/
State Accepted
Headers show

Comments

Yuya Nishihara - Sept. 1, 2019, 1:40 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1567327704 -32400
#      Sun Sep 01 17:48:24 2019 +0900
# Node ID a69f68569113eaa65e3a0e2773d27e2f2dea00bd
# Parent  1da61978931b2f655856fc244613feacbbe2f6f1
rust-cpython: pair leaked reference with its manager object

Still leak_immutable() is unsafe since leak_handle must live longer than
the leaked_ref.

Patch

diff --git a/rust/hg-cpython/src/dirstate/dirs_multiset.rs b/rust/hg-cpython/src/dirstate/dirs_multiset.rs
--- a/rust/hg-cpython/src/dirstate/dirs_multiset.rs
+++ b/rust/hg-cpython/src/dirstate/dirs_multiset.rs
@@ -86,10 +86,11 @@  py_class!(pub class Dirs |py| {
             })
     }
     def __iter__(&self) -> PyResult<DirsMultisetKeysIterator> {
+        let (leak_handle, leaked_ref) = self.leak_immutable(py)?;
         DirsMultisetKeysIterator::create_instance(
             py,
-            RefCell::new(Some(DirsMultisetLeakedRef::new(py, &self))),
-            RefCell::new(Box::new(self.leak_immutable(py)?.iter())),
+            RefCell::new(Some(leak_handle)),
+            RefCell::new(Box::new(leaked_ref.iter())),
         )
     }
 
diff --git a/rust/hg-cpython/src/dirstate/dirstate_map.rs b/rust/hg-cpython/src/dirstate/dirstate_map.rs
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs
+++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs
@@ -319,26 +319,29 @@  py_class!(pub class DirstateMap |py| {
     }
 
     def keys(&self) -> PyResult<DirstateMapKeysIterator> {
+        let (leak_handle, leaked_ref) = self.leak_immutable(py)?;
         DirstateMapKeysIterator::from_inner(
             py,
-            Some(DirstateMapLeakedRef::new(py, &self)),
-            Box::new(self.leak_immutable(py)?.iter()),
+            Some(leak_handle),
+            Box::new(leaked_ref.iter()),
         )
     }
 
     def items(&self) -> PyResult<DirstateMapItemsIterator> {
+        let (leak_handle, leaked_ref) = self.leak_immutable(py)?;
         DirstateMapItemsIterator::from_inner(
             py,
-            Some(DirstateMapLeakedRef::new(py, &self)),
-            Box::new(self.leak_immutable(py)?.iter()),
+            Some(leak_handle),
+            Box::new(leaked_ref.iter()),
         )
     }
 
     def __iter__(&self) -> PyResult<DirstateMapKeysIterator> {
+        let (leak_handle, leaked_ref) = self.leak_immutable(py)?;
         DirstateMapKeysIterator::from_inner(
             py,
-            Some(DirstateMapLeakedRef::new(py, &self)),
-            Box::new(self.leak_immutable(py)?.iter()),
+            Some(leak_handle),
+            Box::new(leaked_ref.iter()),
         )
     }
 
@@ -431,18 +434,20 @@  py_class!(pub class DirstateMap |py| {
     }
 
     def copymapiter(&self) -> PyResult<CopyMapKeysIterator> {
+        let (leak_handle, leaked_ref) = self.leak_immutable(py)?;
         CopyMapKeysIterator::from_inner(
             py,
-            Some(DirstateMapLeakedRef::new(py, &self)),
-            Box::new(self.leak_immutable(py)?.copy_map.iter()),
+            Some(leak_handle),
+            Box::new(leaked_ref.copy_map.iter()),
         )
     }
 
     def copymapitemsiter(&self) -> PyResult<CopyMapItemsIterator> {
+        let (leak_handle, leaked_ref) = self.leak_immutable(py)?;
         CopyMapItemsIterator::from_inner(
             py,
-            Some(DirstateMapLeakedRef::new(py, &self)),
-            Box::new(self.leak_immutable(py)?.copy_map.iter()),
+            Some(leak_handle),
+            Box::new(leaked_ref.copy_map.iter()),
         )
     }
 
diff --git a/rust/hg-cpython/src/ref_sharing.rs b/rust/hg-cpython/src/ref_sharing.rs
--- a/rust/hg-cpython/src/ref_sharing.rs
+++ b/rust/hg-cpython/src/ref_sharing.rs
@@ -216,14 +216,24 @@  macro_rules! py_shared_ref {
                     .borrow_mut(py, unsafe { data.borrow_mut() })
             }
 
+            /// Returns a leaked reference and its management object.
+            ///
+            /// # Safety
+            ///
+            /// It's up to you to make sure that the management object lives
+            /// longer than the leaked reference. Otherwise, you'll get a
+            /// dangling reference.
             fn leak_immutable<'a>(
                 &'a self,
                 py: Python<'a>,
-            ) -> PyResult<&'static $inner_struct> {
+            ) -> PyResult<($leaked, &'static $inner_struct)> {
                 // assert $data_member type
                 use crate::ref_sharing::PySharedRefCell;
                 let data: &PySharedRefCell<_> = self.$data_member(py);
-                self.py_shared_state(py).leak_immutable(py, data)
+                let static_ref =
+                    self.py_shared_state(py).leak_immutable(py, data)?;
+                let leak_handle = $leaked::new(py, self);
+                Ok((leak_handle, static_ref))
             }
         }
 
@@ -336,10 +346,11 @@  macro_rules! py_shared_iterator_impl {
 ///     data py_shared_state: PySharedState;
 ///
 ///     def __iter__(&self) -> PyResult<MyTypeItemsIterator> {
+///         let (leak_handle, leaked_ref) = self.leak_immutable(py)?;
 ///         MyTypeItemsIterator::create_instance(
 ///             py,
-///             RefCell::new(Some(MyTypeLeakedRef::new(py, &self))),
-///             RefCell::new(self.leak_immutable(py).iter()),
+///             RefCell::new(Some(leak_handle)),
+///             RefCell::new(leaked_ref.iter()),
 ///         )
 ///     }
 /// });