##// END OF EJS Templates
rust-cpython: allow mutation unless leaked reference is borrowed...
Yuya Nishihara -
r43606:ed50f2c3 default
parent child Browse files
Show More
@@ -687,8 +687,7 b' class dirstate(object):'
687 687 delaywrite = self._ui.configint(b'debug', b'dirstate.delaywrite')
688 688 if delaywrite > 0:
689 689 # do we have any files to delay for?
690 items = pycompat.iteritems(self._map)
691 for f, e in items:
690 for f, e in pycompat.iteritems(self._map):
692 691 if e[0] == b'n' and e[3] == now:
693 692 import time # to avoid useless import
694 693
@@ -700,12 +699,6 b' class dirstate(object):'
700 699 time.sleep(end - clock)
701 700 now = end # trust our estimate that the end is near now
702 701 break
703 # since the iterator is potentially not deleted,
704 # delete the iterator to release the reference for the Rust
705 # implementation.
706 # TODO make the Rust implementation behave like Python
707 # since this would not work with a non ref-counting GC.
708 del items
709 702
710 703 self._map.write(st, now)
711 704 self._lastnormaltime = 0
@@ -38,17 +38,20 b' use std::sync::atomic::{AtomicUsize, Ord'
38 38 /// `PySharedRefCell` is allowed only through its `borrow_mut()`.
39 39 /// - The `py: Python<'_>` token, which makes sure that any data access is
40 40 /// synchronized by the GIL.
41 /// - The `borrow_count`, which is the number of references borrowed from
42 /// `PyLeaked`. Just like `RefCell`, mutation is prohibited while `PyLeaked`
43 /// is borrowed.
41 44 /// - The `generation` counter, which increments on `borrow_mut()`. `PyLeaked`
42 45 /// reference is valid only if the `current_generation()` equals to the
43 46 /// `generation` at the time of `leak_immutable()`.
44 47 #[derive(Debug, Default)]
45 48 struct PySharedState {
46 leak_count: Cell<usize>,
47 49 mutably_borrowed: Cell<bool>,
48 50 // The counter variable could be Cell<usize> since any operation on
49 51 // PySharedState is synchronized by the GIL, but being "atomic" makes
50 52 // PySharedState inherently Sync. The ordering requirement doesn't
51 53 // matter thanks to the GIL.
54 borrow_count: AtomicUsize,
52 55 generation: AtomicUsize,
53 56 }
54 57
@@ -69,7 +72,7 b' impl PySharedState {'
69 72 mutable reference in a Python object",
70 73 ));
71 74 }
72 match self.leak_count.get() {
75 match self.current_borrow_count(py) {
73 76 0 => {
74 77 self.mutably_borrowed.replace(true);
75 78 // Note that this wraps around to the same value if mutably
@@ -78,21 +81,9 b' impl PySharedState {'
78 81 self.generation.fetch_add(1, Ordering::Relaxed);
79 82 Ok(PyRefMut::new(py, pyrefmut, self))
80 83 }
81 // TODO
82 // For now, this works differently than Python references
83 // in the case of iterators.
84 // Python does not complain when the data an iterator
85 // points to is modified if the iterator is never used
86 // afterwards.
87 // Here, we are stricter than this by refusing to give a
88 // mutable reference if it is already borrowed.
89 // While the additional safety might be argued for, it
90 // breaks valid programming patterns in Python and we need
91 // to fix this issue down the line.
92 84 _ => Err(AlreadyBorrowed::new(
93 85 py,
94 "Cannot borrow mutably while there are \
95 immutable references in Python objects",
86 "Cannot borrow mutably while immutably borrowed",
96 87 )),
97 88 }
98 89 }
@@ -121,23 +112,35 b' impl PySharedState {'
121 112 // can move stuff to PySharedRefCell?
122 113 let ptr = data.as_ptr();
123 114 let state_ptr: *const PySharedState = &data.py_shared_state;
124 self.leak_count.replace(self.leak_count.get() + 1);
125 115 Ok((&*ptr, &*state_ptr))
126 116 }
127 117
118 fn current_borrow_count(&self, _py: Python) -> usize {
119 self.borrow_count.load(Ordering::Relaxed)
120 }
121
122 fn increase_borrow_count(&self, _py: Python) {
123 // Note that this wraps around if there are more than usize::MAX
124 // borrowed references, which shouldn't happen due to memory limit.
125 self.borrow_count.fetch_add(1, Ordering::Relaxed);
126 }
127
128 fn decrease_borrow_count(&self, _py: Python) {
129 let prev_count = self.borrow_count.fetch_sub(1, Ordering::Relaxed);
130 assert!(prev_count > 0);
131 }
132
128 133 /// # Safety
129 134 ///
130 135 /// It's up to you to make sure the reference is about to be deleted
131 136 /// when updating the leak count.
132 fn decrease_leak_count(&self, _py: Python, mutable: bool) {
137 fn decrease_leak_count(&self, py: Python, mutable: bool) {
133 138 if mutable {
134 assert_eq!(self.leak_count.get(), 0);
139 assert_eq!(self.current_borrow_count(py), 0);
135 140 assert!(self.mutably_borrowed.get());
136 141 self.mutably_borrowed.replace(false);
137 142 } else {
138 let count = self.leak_count.get();
139 assert!(count > 0);
140 self.leak_count.replace(count - 1);
143 unimplemented!();
141 144 }
142 145 }
143 146
@@ -146,6 +149,32 b' impl PySharedState {'
146 149 }
147 150 }
148 151
152 /// Helper to keep the borrow count updated while the shared object is
153 /// immutably borrowed without using the `RefCell` interface.
154 struct BorrowPyShared<'a> {
155 py: Python<'a>,
156 py_shared_state: &'a PySharedState,
157 }
158
159 impl<'a> BorrowPyShared<'a> {
160 fn new(
161 py: Python<'a>,
162 py_shared_state: &'a PySharedState,
163 ) -> BorrowPyShared<'a> {
164 py_shared_state.increase_borrow_count(py);
165 BorrowPyShared {
166 py,
167 py_shared_state,
168 }
169 }
170 }
171
172 impl Drop for BorrowPyShared<'_> {
173 fn drop(&mut self) {
174 self.py_shared_state.decrease_borrow_count(self.py);
175 }
176 }
177
149 178 /// `RefCell` wrapper to be safely used in conjunction with `PySharedState`.
150 179 ///
151 180 /// This object can be stored in a `py_class!` object as a data field. Any
@@ -273,13 +302,6 b" impl<'a, T> Drop for PyRefMut<'a, T> {"
273 302 /// Allows a `py_class!` generated struct to share references to one of its
274 303 /// data members with Python.
275 304 ///
276 /// # Warning
277 ///
278 /// TODO allow Python container types: for now, integration with the garbage
279 /// collector does not extend to Rust structs holding references to Python
280 /// objects. Should the need surface, `__traverse__` and `__clear__` will
281 /// need to be written as per the `rust-cpython` docs on GC integration.
282 ///
283 305 /// # Parameters
284 306 ///
285 307 /// * `$name` is the same identifier used in for `py_class!` macro call.
@@ -376,7 +398,7 b' impl<T> PyLeaked<T> {'
376 398 ) -> PyResult<PyLeakedRef<'a, T>> {
377 399 self.validate_generation(py)?;
378 400 Ok(PyLeakedRef {
379 _py: py,
401 _borrow: BorrowPyShared::new(py, self.py_shared_state),
380 402 data: self.data.as_ref().unwrap(),
381 403 })
382 404 }
@@ -393,7 +415,7 b' impl<T> PyLeaked<T> {'
393 415 ) -> PyResult<PyLeakedRefMut<'a, T>> {
394 416 self.validate_generation(py)?;
395 417 Ok(PyLeakedRefMut {
396 _py: py,
418 _borrow: BorrowPyShared::new(py, self.py_shared_state),
397 419 data: self.data.as_mut().unwrap(),
398 420 })
399 421 }
@@ -451,23 +473,9 b' impl<T> PyLeaked<T> {'
451 473 }
452 474 }
453 475
454 impl<T> Drop for PyLeaked<T> {
455 fn drop(&mut self) {
456 // py_shared_state should be alive since we do have
457 // a Python reference to the owner object. Taking GIL makes
458 // sure that the state is only accessed by this thread.
459 let gil = Python::acquire_gil();
460 let py = gil.python();
461 if self.data.is_none() {
462 return; // moved to another PyLeaked
463 }
464 self.py_shared_state.decrease_leak_count(py, false);
465 }
466 }
467
468 476 /// Immutably borrowed reference to a leaked value.
469 477 pub struct PyLeakedRef<'a, T> {
470 _py: Python<'a>,
478 _borrow: BorrowPyShared<'a>,
471 479 data: &'a T,
472 480 }
473 481
@@ -481,7 +489,7 b" impl<T> Deref for PyLeakedRef<'_, T> {"
481 489
482 490 /// Mutably borrowed reference to a leaked value.
483 491 pub struct PyLeakedRefMut<'a, T> {
484 _py: Python<'a>,
492 _borrow: BorrowPyShared<'a>,
485 493 data: &'a mut T,
486 494 }
487 495
@@ -570,6 +578,7 b' macro_rules! py_shared_iterator {'
570 578 let mut iter = leaked.try_borrow_mut(py)?;
571 579 match iter.next() {
572 580 None => {
581 drop(iter);
573 582 // replace Some(inner) by None, drop $leaked
574 583 inner_opt.take();
575 584 Ok(None)
@@ -649,9 +658,7 b' mod test {'
649 658 let (gil, owner) = prepare_env();
650 659 let py = gil.python();
651 660 let leaked = owner.string_shared(py).leak_immutable().unwrap();
652 owner.string(py).py_shared_state.leak_count.replace(0); // XXX cheat
653 661 owner.string_shared(py).borrow_mut().unwrap().clear();
654 owner.string(py).py_shared_state.leak_count.replace(1); // XXX cheat
655 662 assert!(leaked.try_borrow(py).is_err());
656 663 }
657 664
@@ -661,9 +668,7 b' mod test {'
661 668 let py = gil.python();
662 669 let leaked = owner.string_shared(py).leak_immutable().unwrap();
663 670 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 671 owner.string_shared(py).borrow_mut().unwrap().clear();
666 owner.string(py).py_shared_state.leak_count.replace(1); // XXX cheat
667 672 assert!(leaked_iter.try_borrow_mut(py).is_err());
668 673 }
669 674
@@ -673,19 +678,39 b' mod test {'
673 678 let (gil, owner) = prepare_env();
674 679 let py = gil.python();
675 680 let leaked = owner.string_shared(py).leak_immutable().unwrap();
676 owner.string(py).py_shared_state.leak_count.replace(0); // XXX cheat
677 681 owner.string_shared(py).borrow_mut().unwrap().clear();
678 owner.string(py).py_shared_state.leak_count.replace(1); // XXX cheat
679 682 let _leaked_iter = unsafe { leaked.map(py, |s| s.chars()) };
680 683 }
681 684
682 685 #[test]
683 fn test_borrow_mut_while_leaked() {
686 fn test_borrow_mut_while_leaked_ref() {
684 687 let (gil, owner) = prepare_env();
685 688 let py = gil.python();
686 689 assert!(owner.string_shared(py).borrow_mut().is_ok());
687 let _leaked = owner.string_shared(py).leak_immutable().unwrap();
688 // TODO: will be allowed
689 assert!(owner.string_shared(py).borrow_mut().is_err());
690 let leaked = owner.string_shared(py).leak_immutable().unwrap();
691 {
692 let _leaked_ref = leaked.try_borrow(py).unwrap();
693 assert!(owner.string_shared(py).borrow_mut().is_err());
694 {
695 let _leaked_ref2 = leaked.try_borrow(py).unwrap();
696 assert!(owner.string_shared(py).borrow_mut().is_err());
697 }
698 assert!(owner.string_shared(py).borrow_mut().is_err());
699 }
700 assert!(owner.string_shared(py).borrow_mut().is_ok());
701 }
702
703 #[test]
704 fn test_borrow_mut_while_leaked_ref_mut() {
705 let (gil, owner) = prepare_env();
706 let py = gil.python();
707 assert!(owner.string_shared(py).borrow_mut().is_ok());
708 let leaked = owner.string_shared(py).leak_immutable().unwrap();
709 let mut leaked_iter = unsafe { leaked.map(py, |s| s.chars()) };
710 {
711 let _leaked_ref = leaked_iter.try_borrow_mut(py).unwrap();
712 assert!(owner.string_shared(py).borrow_mut().is_err());
713 }
714 assert!(owner.string_shared(py).borrow_mut().is_ok());
690 715 }
691 716 }
General Comments 0
You need to be logged in to leave comments. Login now