Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make gil tests not racy #2190

Merged
merged 2 commits into from
Mar 3, 2022
Merged

Make gil tests not racy #2190

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9b73f2
Make tests not racy
mejrs Feb 26, 2022
67b77ce
be a bit more resilient to other tests
mejrs Mar 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 98 additions & 50 deletions src/gil.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,71 +700,119 @@ mod tests {

#[test]
fn test_clone_without_gil() {
let gil = Python::acquire_gil();
let py = gil.python();
let obj = get_object(py);
let count = obj.get_refcnt(py);
use crate::{Py, PyAny};
use std::sync::atomic::{AtomicBool, Ordering};
use std::{sync::Arc, thread};

// Some spinlocks for synchronizing
static GIL_ACQUIRED: AtomicBool = AtomicBool::new(false);
static OBJECT_CLONED: AtomicBool = AtomicBool::new(false);
static REFCNT_CHECKED: AtomicBool = AtomicBool::new(false);

Python::with_gil(|py| {
let obj: Arc<Py<PyAny>> = Arc::new(get_object(py));
let thread_obj = Arc::clone(&obj);

let count = (&*obj).get_refcnt(py);
println!(
"1: The object has been created and its reference count is {}",
count
);

// Cloning without GIL should not update reference count
drop(gil);
let c = obj.clone();
assert_eq!(
count,
obj.get_refcnt(unsafe { Python::assume_gil_acquired() })
);
let handle = thread::spawn(move || {
Python::with_gil(move |py| {
println!("3. The GIL has been acquired on another thread.");
GIL_ACQUIRED.store(true, Ordering::Release);

// Acquring GIL will clear this pending change
let gil = Python::acquire_gil();
let py = gil.python();
// Spin a bit while the main thread registers obj in POOL
while !OBJECT_CLONED.load(Ordering::Acquire) {}
println!("5. Checking refcnt");
assert_eq!(thread_obj.get_refcnt(py), count);

REFCNT_CHECKED.store(true, Ordering::Release);
})
});

let cloned = py.allow_threads(|| {
println!("2. The GIL has been released.");

// Total reference count should be one higher
assert_eq!(count + 1, obj.get_refcnt(py));
// spin until the gil has been acquired on the thread.
while !GIL_ACQUIRED.load(Ordering::Acquire) {}

// Clone dropped
drop(c);
println!("4. The other thread is now hogging the GIL, we clone without it held");
// Cloning without GIL should not update reference count
let cloned = Py::clone(&*obj);
OBJECT_CLONED.store(true, Ordering::Release);
cloned
});

while !REFCNT_CHECKED.load(Ordering::Acquire) {}

// Returning from allow_threads doesn't clear the pool
py.allow_threads(|| {
// Acquiring GIL will clear the pending change
Python::with_gil(|_| {});
});

// Overall count is now back to the original, and should be no pending change
assert_eq!(count, obj.get_refcnt(py));
println!("6. The main thread has acquired the GIL again and processed the pool.");

// Total reference count should be one higher
assert_eq!(obj.get_refcnt(py), count + 1);

// Clone dropped
drop(cloned);
// Ensure refcount of the arc is 1
handle.join().unwrap();

// Overall count is now back to the original, and should be no pending change
assert_eq!(Arc::try_unwrap(obj).unwrap().get_refcnt(py), count);
});
}

#[test]
fn test_clone_in_other_thread() {
let gil = Python::acquire_gil();
let py = gil.python();
let obj = get_object(py);
let count = obj.get_refcnt(py);

// Move obj to a thread which does not have the GIL, and clone it
let t = std::thread::spawn(move || {
// Cloning without GIL should not update reference count
#[allow(clippy::redundant_clone)]
let _ = obj.clone();
assert_eq!(
count,
obj.get_refcnt(unsafe { Python::assume_gil_acquired() })
);
use crate::Py;
use std::sync::atomic::{AtomicBool, Ordering};
use std::{sync::Arc, thread};

// Some spinlocks for synchronizing
static OBJECT_CLONED: AtomicBool = AtomicBool::new(false);

let (obj, count, ptr) = Python::with_gil(|py| {
let obj = Arc::new(get_object(py));
let count = obj.get_refcnt(py);
let thread_obj = Arc::clone(&obj);

// Start a thread which does not have the GIL, and clone it
let t = thread::spawn(move || {
// Cloning without GIL should not update reference count
#[allow(clippy::redundant_clone)]
let _ = Py::clone(&*thread_obj);
OBJECT_CLONED.store(true, Ordering::Release);
});

// Return obj so original thread can continue to use
obj
});
while !OBJECT_CLONED.load(Ordering::Acquire) {}
assert_eq!(count, obj.get_refcnt(py));

let obj = t.join().unwrap();
let ptr = NonNull::new(obj.as_ptr()).unwrap();
t.join().unwrap();
let ptr = NonNull::new(obj.as_ptr()).unwrap();

// The pointer should appear once in the incref pool, and once in the
// decref pool (for the clone being created and also dropped)
assert_eq!(&POOL.pointer_ops.lock().0, &vec![ptr]);
assert_eq!(&POOL.pointer_ops.lock().1, &vec![ptr]);
// The pointer should appear once in the incref pool, and once in the
// decref pool (for the clone being created and also dropped)
assert!(POOL.pointer_ops.lock().0.contains(&ptr));
assert!(POOL.pointer_ops.lock().1.contains(&ptr));

// Re-acquring GIL will clear these pending changes
drop(gil);
let gil = Python::acquire_gil();
(obj, count, ptr)
});

assert!(POOL.pointer_ops.lock().0.is_empty());
assert!(POOL.pointer_ops.lock().1.is_empty());
Python::with_gil(|py| {
// Acquiring the gil clears the pool
assert!(!POOL.pointer_ops.lock().0.contains(&ptr));
assert!(!POOL.pointer_ops.lock().1.contains(&ptr));

// Overall count is still unchanged
assert_eq!(count, obj.get_refcnt(gil.python()));
// Overall count is still unchanged
assert_eq!(count, obj.get_refcnt(py));
});
}

#[test]
Expand Down