Skip to content

Commit

Permalink
Fix tests for cargo stress on free-threaded build (#4469)
Browse files Browse the repository at this point in the history
* make it possible to run test_bad_datetime_module_panic under 'cargo stress'

* weaken assertions in tests of the pending decrefs pool on free-threaded builds

* use the tempfile crate to create a temporary directory
  • Loading branch information
ngoldbaum committed Aug 23, 2024
1 parent a8970ea commit ec0853d
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 8 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0.61"
rayon = "1.6.1"
futures = "0.3.28"
tempfile = "3.12.0"

[build-dependencies]
pyo3-build-config = { path = "pyo3-build-config", version = "=0.23.0-dev", features = ["resolve-config"] }
Expand Down
14 changes: 12 additions & 2 deletions src/gil.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,9 @@ mod tests {
.contains(&unsafe { NonNull::new_unchecked(obj.as_ptr()) })
}

#[cfg(not(pyo3_disable_reference_pool))]
// with no GIL, threads can empty the POOL at any time, so this
// function does not test anything meaningful
#[cfg(not(any(pyo3_disable_reference_pool, Py_GIL_DISABLED)))]
fn pool_dec_refs_contains(obj: &PyObject) -> bool {
POOL.pending_decrefs
.lock()
Expand All @@ -471,7 +473,7 @@ mod tests {
drop(reference);

assert_eq!(obj.get_refcnt(py), 1);
#[cfg(not(pyo3_disable_reference_pool))]
#[cfg(not(any(pyo3_disable_reference_pool)))]
assert!(pool_dec_refs_does_not_contain(&obj));
});
}
Expand All @@ -493,12 +495,18 @@ mod tests {
// The reference count should not have changed (the GIL has always
// been held by this thread), it is remembered to release later.
assert_eq!(obj.get_refcnt(py), 2);
#[cfg(not(Py_GIL_DISABLED))]
assert!(pool_dec_refs_contains(&obj));
obj
});

// Next time the GIL is acquired, the reference is released
#[allow(unused)]
Python::with_gil(|py| {
// with no GIL, another thread could still be processing
// DECREFs after releasing the lock on the POOL, so the
// refcnt could still be 2 when this assert happens
#[cfg(not(Py_GIL_DISABLED))]
assert_eq!(obj.get_refcnt(py), 1);
assert!(pool_dec_refs_does_not_contain(&obj));
});
Expand Down Expand Up @@ -641,13 +649,15 @@ mod tests {
// For GILGuard::acquire

POOL.register_decref(NonNull::new(obj.clone_ref(py).into_ptr()).unwrap());
#[cfg(not(Py_GIL_DISABLED))]
assert!(pool_dec_refs_contains(&obj));
let _guard = GILGuard::acquire();
assert!(pool_dec_refs_does_not_contain(&obj));

// For GILGuard::assume

POOL.register_decref(NonNull::new(obj.clone_ref(py).into_ptr()).unwrap());
#[cfg(not(Py_GIL_DISABLED))]
assert!(pool_dec_refs_contains(&obj));
let _guard2 = unsafe { GILGuard::assume() };
assert!(pool_dec_refs_does_not_contain(&obj));
Expand Down
14 changes: 8 additions & 6 deletions tests/test_datetime_import.rs
Original file line number Diff line number Diff line change
@@ -1,26 +1,28 @@
#![cfg(not(Py_LIMITED_API))]

use pyo3::{prelude::*, types::PyDate};
use tempfile::Builder;

#[test]
#[should_panic(expected = "module 'datetime' has no attribute 'datetime_CAPI'")]
fn test_bad_datetime_module_panic() {
// Create an empty temporary directory
// with an empty "datetime" module which we'll put on the sys.path
let tmpdir = std::env::temp_dir();
let tmpdir = tmpdir.join("pyo3_test_date_check");
let _ = std::fs::remove_dir_all(&tmpdir);
std::fs::create_dir(&tmpdir).unwrap();
std::fs::File::create(tmpdir.join("datetime.py")).unwrap();
let tmpdir = Builder::new()
.prefix("pyo3_test_data_check")
.tempdir()
.unwrap();
std::fs::File::create(tmpdir.path().join("datetime.py")).unwrap();

Python::with_gil(|py: Python<'_>| {
let sys = py.import("sys").unwrap();
sys.getattr("path")
.unwrap()
.call_method1("insert", (0, tmpdir))
.call_method1("insert", (0, tmpdir.path()))
.unwrap();

// This should panic because the "datetime" module is empty
PyDate::new(py, 2018, 1, 1).unwrap();
});
tmpdir.close().unwrap();
}

0 comments on commit ec0853d

Please sign in to comment.