Mercurial > hg
comparison rust/hg-cpython/src/ref_sharing.rs @ 43424:0836efe4967b
rust-cpython: add generation counter to leaked reference
This counter increments on borrow_mut() to invalidate existing leaked
references. This is modeled after the iterator invalidation in Python.
The other checks will be adjusted by the subsequent patches.
author | Yuya Nishihara <yuya@tcha.org> |
---|---|
date | Sat, 05 Oct 2019 08:27:57 -0400 |
parents | 945d4dba5e78 |
children | ed50f2c31a4c |
comparison
equal
deleted
inserted
replaced
43423:945d4dba5e78 | 43424:0836efe4967b |
---|---|
21 // IN THE SOFTWARE. | 21 // IN THE SOFTWARE. |
22 | 22 |
23 //! Macros for use in the `hg-cpython` bridge library. | 23 //! Macros for use in the `hg-cpython` bridge library. |
24 | 24 |
25 use crate::exceptions::AlreadyBorrowed; | 25 use crate::exceptions::AlreadyBorrowed; |
26 use cpython::{PyClone, PyObject, PyResult, Python}; | 26 use cpython::{exc, PyClone, PyErr, PyObject, PyResult, Python}; |
27 use std::cell::{Cell, Ref, RefCell, RefMut}; | 27 use std::cell::{Cell, Ref, RefCell, RefMut}; |
28 use std::ops::{Deref, DerefMut}; | 28 use std::ops::{Deref, DerefMut}; |
29 use std::sync::atomic::{AtomicUsize, Ordering}; | |
29 | 30 |
30 /// Manages the shared state between Python and Rust | 31 /// Manages the shared state between Python and Rust |
32 /// | |
33 /// `PySharedState` is owned by `PySharedRefCell`, and is shared across its | |
34 /// derived references. The consistency of these references are guaranteed | |
35 /// as follows: | |
36 /// | |
37 /// - The immutability of `py_class!` object fields. Any mutation of | |
38 /// `PySharedRefCell` is allowed only through its `borrow_mut()`. | |
39 /// - The `py: Python<'_>` token, which makes sure that any data access is | |
40 /// synchronized by the GIL. | |
41 /// - The `generation` counter, which increments on `borrow_mut()`. `PyLeaked` | |
42 /// reference is valid only if the `current_generation()` equals to the | |
43 /// `generation` at the time of `leak_immutable()`. | |
31 #[derive(Debug, Default)] | 44 #[derive(Debug, Default)] |
32 struct PySharedState { | 45 struct PySharedState { |
33 leak_count: Cell<usize>, | 46 leak_count: Cell<usize>, |
34 mutably_borrowed: Cell<bool>, | 47 mutably_borrowed: Cell<bool>, |
48 // The counter variable could be Cell<usize> since any operation on | |
49 // PySharedState is synchronized by the GIL, but being "atomic" makes | |
50 // PySharedState inherently Sync. The ordering requirement doesn't | |
51 // matter thanks to the GIL. | |
52 generation: AtomicUsize, | |
35 } | 53 } |
36 | 54 |
37 // &PySharedState can be Send because any access to inner cells is | 55 // &PySharedState can be Send because any access to inner cells is |
38 // synchronized by the GIL. | 56 // synchronized by the GIL. |
39 unsafe impl Sync for PySharedState {} | 57 unsafe impl Sync for PySharedState {} |
52 )); | 70 )); |
53 } | 71 } |
54 match self.leak_count.get() { | 72 match self.leak_count.get() { |
55 0 => { | 73 0 => { |
56 self.mutably_borrowed.replace(true); | 74 self.mutably_borrowed.replace(true); |
75 // Note that this wraps around to the same value if mutably | |
76 // borrowed more than usize::MAX times, which wouldn't happen | |
77 // in practice. | |
78 self.generation.fetch_add(1, Ordering::Relaxed); | |
57 Ok(PyRefMut::new(py, pyrefmut, self)) | 79 Ok(PyRefMut::new(py, pyrefmut, self)) |
58 } | 80 } |
59 // TODO | 81 // TODO |
60 // For now, this works differently than Python references | 82 // For now, this works differently than Python references |
61 // in the case of iterators. | 83 // in the case of iterators. |
115 } else { | 137 } else { |
116 let count = self.leak_count.get(); | 138 let count = self.leak_count.get(); |
117 assert!(count > 0); | 139 assert!(count > 0); |
118 self.leak_count.replace(count - 1); | 140 self.leak_count.replace(count - 1); |
119 } | 141 } |
142 } | |
143 | |
144 fn current_generation(&self, _py: Python) -> usize { | |
145 self.generation.load(Ordering::Relaxed) | |
120 } | 146 } |
121 } | 147 } |
122 | 148 |
123 /// `RefCell` wrapper to be safely used in conjunction with `PySharedState`. | 149 /// `RefCell` wrapper to be safely used in conjunction with `PySharedState`. |
124 /// | 150 /// |
306 } | 332 } |
307 }; | 333 }; |
308 } | 334 } |
309 | 335 |
310 /// Manage immutable references to `PyObject` leaked into Python iterators. | 336 /// Manage immutable references to `PyObject` leaked into Python iterators. |
337 /// | |
338 /// This reference will be invalidated once the original value is mutably | |
339 /// borrowed. | |
311 pub struct PyLeaked<T> { | 340 pub struct PyLeaked<T> { |
312 inner: PyObject, | 341 inner: PyObject, |
313 data: Option<T>, | 342 data: Option<T>, |
314 py_shared_state: &'static PySharedState, | 343 py_shared_state: &'static PySharedState, |
344 /// Generation counter of data `T` captured when PyLeaked is created. | |
345 generation: usize, | |
315 } | 346 } |
316 | 347 |
317 // DO NOT implement Deref for PyLeaked<T>! Dereferencing PyLeaked | 348 // DO NOT implement Deref for PyLeaked<T>! Dereferencing PyLeaked |
318 // without taking Python GIL wouldn't be safe. | 349 // without taking Python GIL wouldn't be safe. Also, the underling reference |
350 // is invalid if generation != py_shared_state.generation. | |
319 | 351 |
320 impl<T> PyLeaked<T> { | 352 impl<T> PyLeaked<T> { |
321 /// # Safety | 353 /// # Safety |
322 /// | 354 /// |
323 /// The `py_shared_state` must be owned by the `inner` Python object. | 355 /// The `py_shared_state` must be owned by the `inner` Python object. |
329 ) -> Self { | 361 ) -> Self { |
330 Self { | 362 Self { |
331 inner: inner.clone_ref(py), | 363 inner: inner.clone_ref(py), |
332 data: Some(data), | 364 data: Some(data), |
333 py_shared_state, | 365 py_shared_state, |
366 generation: py_shared_state.current_generation(py), | |
334 } | 367 } |
335 } | 368 } |
336 | 369 |
337 /// Immutably borrows the wrapped value. | 370 /// Immutably borrows the wrapped value. |
371 /// | |
372 /// Borrowing fails if the underlying reference has been invalidated. | |
338 pub fn try_borrow<'a>( | 373 pub fn try_borrow<'a>( |
339 &'a self, | 374 &'a self, |
340 py: Python<'a>, | 375 py: Python<'a>, |
341 ) -> PyResult<PyLeakedRef<'a, T>> { | 376 ) -> PyResult<PyLeakedRef<'a, T>> { |
377 self.validate_generation(py)?; | |
342 Ok(PyLeakedRef { | 378 Ok(PyLeakedRef { |
343 _py: py, | 379 _py: py, |
344 data: self.data.as_ref().unwrap(), | 380 data: self.data.as_ref().unwrap(), |
345 }) | 381 }) |
346 } | 382 } |
347 | 383 |
348 /// Mutably borrows the wrapped value. | 384 /// Mutably borrows the wrapped value. |
385 /// | |
386 /// Borrowing fails if the underlying reference has been invalidated. | |
349 /// | 387 /// |
350 /// Typically `T` is an iterator. If `T` is an immutable reference, | 388 /// Typically `T` is an iterator. If `T` is an immutable reference, |
351 /// `get_mut()` is useless since the inner value can't be mutated. | 389 /// `get_mut()` is useless since the inner value can't be mutated. |
352 pub fn try_borrow_mut<'a>( | 390 pub fn try_borrow_mut<'a>( |
353 &'a mut self, | 391 &'a mut self, |
354 py: Python<'a>, | 392 py: Python<'a>, |
355 ) -> PyResult<PyLeakedRefMut<'a, T>> { | 393 ) -> PyResult<PyLeakedRefMut<'a, T>> { |
394 self.validate_generation(py)?; | |
356 Ok(PyLeakedRefMut { | 395 Ok(PyLeakedRefMut { |
357 _py: py, | 396 _py: py, |
358 data: self.data.as_mut().unwrap(), | 397 data: self.data.as_mut().unwrap(), |
359 }) | 398 }) |
360 } | 399 } |
361 | 400 |
362 /// Converts the inner value by the given function. | 401 /// Converts the inner value by the given function. |
363 /// | 402 /// |
364 /// Typically `T` is a static reference to a container, and `U` is an | 403 /// Typically `T` is a static reference to a container, and `U` is an |
365 /// iterator of that container. | 404 /// iterator of that container. |
405 /// | |
406 /// # Panics | |
407 /// | |
408 /// Panics if the underlying reference has been invalidated. | |
409 /// | |
410 /// This is typically called immediately after the `PyLeaked` is obtained. | |
411 /// In which case, the reference must be valid and no panic would occur. | |
366 /// | 412 /// |
367 /// # Safety | 413 /// # Safety |
368 /// | 414 /// |
369 /// The lifetime of the object passed in to the function `f` is cheated. | 415 /// The lifetime of the object passed in to the function `f` is cheated. |
370 /// It's typically a static reference, but is valid only while the | 416 /// It's typically a static reference, but is valid only while the |
373 pub unsafe fn map<U>( | 419 pub unsafe fn map<U>( |
374 mut self, | 420 mut self, |
375 py: Python, | 421 py: Python, |
376 f: impl FnOnce(T) -> U, | 422 f: impl FnOnce(T) -> U, |
377 ) -> PyLeaked<U> { | 423 ) -> PyLeaked<U> { |
424 // Needs to test the generation value to make sure self.data reference | |
425 // is still intact. | |
426 self.validate_generation(py) | |
427 .expect("map() over invalidated leaked reference"); | |
428 | |
378 // f() could make the self.data outlive. That's why map() is unsafe. | 429 // f() could make the self.data outlive. That's why map() is unsafe. |
379 // In order to make this function safe, maybe we'll need a way to | 430 // In order to make this function safe, maybe we'll need a way to |
380 // temporarily restrict the lifetime of self.data and translate the | 431 // temporarily restrict the lifetime of self.data and translate the |
381 // returned object back to Something<'static>. | 432 // returned object back to Something<'static>. |
382 let new_data = f(self.data.take().unwrap()); | 433 let new_data = f(self.data.take().unwrap()); |
383 PyLeaked { | 434 PyLeaked { |
384 inner: self.inner.clone_ref(py), | 435 inner: self.inner.clone_ref(py), |
385 data: Some(new_data), | 436 data: Some(new_data), |
386 py_shared_state: self.py_shared_state, | 437 py_shared_state: self.py_shared_state, |
438 generation: self.generation, | |
439 } | |
440 } | |
441 | |
442 fn validate_generation(&self, py: Python) -> PyResult<()> { | |
443 if self.py_shared_state.current_generation(py) == self.generation { | |
444 Ok(()) | |
445 } else { | |
446 Err(PyErr::new::<exc::RuntimeError, _>( | |
447 py, | |
448 "Cannot access to leaked reference after mutation", | |
449 )) | |
387 } | 450 } |
388 } | 451 } |
389 } | 452 } |
390 | 453 |
391 impl<T> Drop for PyLeaked<T> { | 454 impl<T> Drop for PyLeaked<T> { |
580 assert_eq!(leaked_ref.next(), Some('w')); | 643 assert_eq!(leaked_ref.next(), Some('w')); |
581 assert_eq!(leaked_ref.next(), None); | 644 assert_eq!(leaked_ref.next(), None); |
582 } | 645 } |
583 | 646 |
584 #[test] | 647 #[test] |
648 fn test_leaked_borrow_after_mut() { | |
649 let (gil, owner) = prepare_env(); | |
650 let py = gil.python(); | |
651 let leaked = owner.string_shared(py).leak_immutable().unwrap(); | |
652 owner.string(py).py_shared_state.leak_count.replace(0); // XXX cheat | |
653 owner.string_shared(py).borrow_mut().unwrap().clear(); | |
654 owner.string(py).py_shared_state.leak_count.replace(1); // XXX cheat | |
655 assert!(leaked.try_borrow(py).is_err()); | |
656 } | |
657 | |
658 #[test] | |
659 fn test_leaked_borrow_mut_after_mut() { | |
660 let (gil, owner) = prepare_env(); | |
661 let py = gil.python(); | |
662 let leaked = owner.string_shared(py).leak_immutable().unwrap(); | |
663 let mut leaked_iter = unsafe { leaked.map(py, |s| s.chars()) }; | |
664 owner.string(py).py_shared_state.leak_count.replace(0); // XXX cheat | |
665 owner.string_shared(py).borrow_mut().unwrap().clear(); | |
666 owner.string(py).py_shared_state.leak_count.replace(1); // XXX cheat | |
667 assert!(leaked_iter.try_borrow_mut(py).is_err()); | |
668 } | |
669 | |
670 #[test] | |
671 #[should_panic(expected = "map() over invalidated leaked reference")] | |
672 fn test_leaked_map_after_mut() { | |
673 let (gil, owner) = prepare_env(); | |
674 let py = gil.python(); | |
675 let leaked = owner.string_shared(py).leak_immutable().unwrap(); | |
676 owner.string(py).py_shared_state.leak_count.replace(0); // XXX cheat | |
677 owner.string_shared(py).borrow_mut().unwrap().clear(); | |
678 owner.string(py).py_shared_state.leak_count.replace(1); // XXX cheat | |
679 let _leaked_iter = unsafe { leaked.map(py, |s| s.chars()) }; | |
680 } | |
681 | |
682 #[test] | |
585 fn test_borrow_mut_while_leaked() { | 683 fn test_borrow_mut_while_leaked() { |
586 let (gil, owner) = prepare_env(); | 684 let (gil, owner) = prepare_env(); |
587 let py = gil.python(); | 685 let py = gil.python(); |
588 assert!(owner.string_shared(py).borrow_mut().is_ok()); | 686 assert!(owner.string_shared(py).borrow_mut().is_ok()); |
589 let _leaked = owner.string_shared(py).leak_immutable().unwrap(); | 687 let _leaked = owner.string_shared(py).leak_immutable().unwrap(); |