Patchwork [04,of,10] rust-cpython: store leaked reference to PySharedState in $leaked struct

login
register
mail settings
Submitter Yuya Nishihara
Date Sept. 22, 2019, 6:41 a.m.
Message ID <cec8317e90342e922cfc.1569134501@mimosa>
Download mbox | patch
Permalink /patch/41723/
State Accepted
Headers show

Comments

Yuya Nishihara - Sept. 22, 2019, 6:41 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1568531085 -32400
#      Sun Sep 15 16:04:45 2019 +0900
# Node ID cec8317e90342e922cfc8228aa268135ad13d26f
# Parent  fcaf01804ac8198d52d141d642d5ef7c935360cc
rust-cpython: store leaked reference to PySharedState in $leaked struct

I want to move it out of the macro, and allow multiple sharable objects
per PyObject.

Patch

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
@@ -59,8 +59,8 @@  impl PySharedState {
         }
     }
 
-    /// Return a reference to the wrapped data with an artificial static
-    /// lifetime.
+    /// Return a reference to the wrapped data and its state with an
+    /// artificial static lifetime.
     /// We need to be protected by the GIL for thread-safety.
     ///
     /// # Safety
@@ -71,7 +71,7 @@  impl PySharedState {
         &self,
         py: Python,
         data: &PySharedRefCell<T>,
-    ) -> PyResult<&'static T> {
+    ) -> PyResult<(&'static T, &'static PySharedState)> {
         if self.mutably_borrowed.get() {
             return Err(AlreadyBorrowed::new(
                 py,
@@ -79,9 +79,12 @@  impl PySharedState {
                  mutable reference in Python objects",
             ));
         }
+        // TODO: it's weird that self is data.py_shared_state. Maybe we
+        // can move stuff to PySharedRefCell?
         let ptr = data.as_ptr();
+        let state_ptr: *const PySharedState = &data.py_shared_state;
         self.leak_count.replace(self.leak_count.get() + 1);
-        Ok(&*ptr)
+        Ok((&*ptr, &*state_ptr))
     }
 
     /// # Safety
@@ -252,9 +255,9 @@  macro_rules! py_shared_ref {
                 // assert $data_member type
                 use crate::ref_sharing::PySharedRefCell;
                 let data: &PySharedRefCell<_> = self.$data_member(py);
-                let static_ref =
+                let (static_ref, static_state_ref) =
                     data.py_shared_state.leak_immutable(py, data)?;
-                let leak_handle = $leaked::new(py, self);
+                let leak_handle = $leaked::new(py, self, static_state_ref);
                 Ok((leak_handle, static_ref))
             }
         }
@@ -265,26 +268,38 @@  macro_rules! py_shared_ref {
         /// In truth, this does not represent leaked references themselves;
         /// it is instead useful alongside them to manage them.
         pub struct $leaked {
-            inner: $name,
+            _inner: $name,
+            py_shared_state: &'static crate::ref_sharing::PySharedState,
         }
 
         impl $leaked {
+            /// # Safety
+            ///
+            /// The `py_shared_state` must be owned by the `inner` Python
+            /// object.
             // Marked as unsafe so client code wouldn't construct $leaked
             // struct by mistake. Its drop() is unsafe.
-            unsafe fn new(py: Python, inner: &$name) -> Self {
+            unsafe fn new(
+                py: Python,
+                inner: &$name,
+                py_shared_state: &'static crate::ref_sharing::PySharedState,
+            ) -> Self {
                 Self {
-                    inner: inner.clone_ref(py),
+                    _inner: inner.clone_ref(py),
+                    py_shared_state,
                 }
             }
         }
 
         impl Drop for $leaked {
             fn drop(&mut self) {
+                // py_shared_state should be alive since we do have
+                // a Python reference to the owner object. Taking GIL makes
+                // sure that the state is only accessed by this thread.
                 let gil = Python::acquire_gil();
                 let py = gil.python();
-                let state = &self.inner.$data_member(py).py_shared_state;
                 unsafe {
-                    state.decrease_leak_count(py, false);
+                    self.py_shared_state.decrease_leak_count(py, false);
                 }
             }
         }