Commit 9d1bd3a1 authored by Georges Racinet's avatar Georges Racinet
Browse files

Non compiling attempt to reuse cell::Ref for safety locking

(could be simplified)
This illustrates that it's a bit tricky: it's tempting to leverage
the RefCell we have to forbid mutation of the HashSet while an iterator
on it is active. The idea would be that if the iterator has one of those
Refs, simply by droping it, it would decrement the immutable borrow counter
from the main RustSet RefCell

But this doesn't work, because cell::Ref is not Send, hence can't be used
as data within `py_class!`. Here, it would have been safe, though, because
no Rust code will interfere with these while they are leaked to the Python
side and the Python side is protected by the GIL.
We'll have to implement an higher level locking system
parent fa1138b46f57
......@@ -10,7 +10,7 @@
extern crate cpython;
use cpython::*;
use std::cell::RefCell;
use std::cell::{Ref, RefCell};
use std::collections::hash_set::Iter;
use std::collections::HashSet;
......@@ -41,13 +41,20 @@ py_class!(class RustSet |py| {
}
def __iter__(&self) -> PyResult<RustSetIterator> {
// solution faire un &'static sur le refcell
// et ensuite seulement appeler borrow
let cell = self.hs(py) as *const RefCell<Inner>;
let cell_static: &'static RefCell<Inner> = unsafe {&* cell};
let cellref = cell_static.borrow();
let ptr = self.hs(py).as_ptr();
let as_static: &'static Inner = unsafe {&*ptr};
RustSetIterator::create_instance(
py,
self.clone_ref(py),
RefCell::new(as_static.iter()))
RefCell::new(as_static.iter()),
cellref)
}
def clear(&self) -> PyResult<PyObject> {
......@@ -61,10 +68,12 @@ py_class!(class RustSet |py| {
});
py_class!(class RustSetIterator |py| {
data hs: RustSet;
data it: RefCell<Iter<'static, u32>>;
// `setref` represents the reference that we have otherwise leaked
// in `it` by changing its lifetime to `'static`
data setref: Ref<'static, Inner>;
def __next__(&self) -> PyResult<Option<u32>> {
Ok(self.it(py).borrow_mut().next().map(|r| *r))
......@@ -76,7 +85,6 @@ py_class!(class RustSetIterator |py| {
});
py_module_initializer!(shared_ref, initshared_ref, PyInit_shared_ref, |py, m| {
m.add(
py,
......
......@@ -45,9 +45,18 @@ def test_race_safety():
it = iter(rs)
# Trigger freeing the underlying memory
rs.clear()
try:
rs.clear()
except SystemError as exc:
# TODO proper exception instead of panic
assert 'Rust panic' in str(exc)
else:
raise AssertionError("Should not have been able to clear RustSet "
"instance while holding an iterator on it")
next(it) # that would be a segfault
next(it) # segfault
del it
rs.clear()
def run():
test_basic()
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment