From 3d699b371de39a8fa6effbfc9d66d92159089726 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 24 Sep 2024 12:01:05 -0600 Subject: [PATCH 1/2] fix flaky tests on free-threaded build --- tests/test_gc.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/tests/test_gc.rs b/tests/test_gc.rs index a9380c29f7d..f9f6d508746 100644 --- a/tests/test_gc.rs +++ b/tests/test_gc.rs @@ -34,6 +34,16 @@ fn class_with_freelist() { }); } +/// Tests that drop is eventually called on objects that are dropped when the +/// GIL is not held. +/// +/// +/// On the free-threaded build, threads are resumed before tp_clear() calls +/// finish, so drop might not necessarily be called when a test checks. We +/// therefore cannot check that behavior in the free-threaded build without +/// introducing a flaky test +/// +/// See https://peps.python.org/pep-0703/#stop-the-world struct TestDropCall { drop_called: Arc, } @@ -71,7 +81,9 @@ fn data_is_dropped() { drop(inst); }); + #[cfg(not(Py_GIL_DISABLED))] assert!(drop_called1.load(Ordering::Relaxed)); + #[cfg(not(Py_GIL_DISABLED))] assert!(drop_called2.load(Ordering::Relaxed)); } @@ -120,9 +132,6 @@ fn gc_integration() { Python::with_gil(|py| { py.run(ffi::c_str!("import gc; gc.collect()"), None, None) .unwrap(); - // threads are resumed before tp_clear() calls finish, so drop might not - // necessarily be called when we get here see - // https://peps.python.org/pep-0703/#stop-the-world #[cfg(not(Py_GIL_DISABLED))] assert!(drop_called.load(Ordering::Relaxed)); }); @@ -229,7 +238,9 @@ fn inheritance_with_new_methods_with_drop() { base.data = Some(Arc::clone(&drop_called2)); }); + #[cfg(not(Py_GIL_DISABLED))] assert!(drop_called1.load(Ordering::Relaxed)); + #[cfg(not(Py_GIL_DISABLED))] assert!(drop_called2.load(Ordering::Relaxed)); } @@ -482,6 +493,7 @@ fn drop_during_traversal_with_gil() { .unwrap(); }); } + #[cfg(not(Py_GIL_DISABLED))] assert!(drop_called.load(Ordering::Relaxed)); } @@ -517,6 +529,7 @@ fn drop_during_traversal_without_gil() { .unwrap(); }); } + #[cfg(not(Py_GIL_DISABLED))] assert!(drop_called.load(Ordering::Relaxed)); } From 87f45f2cdbe2569a6914f2f1bebb269768bdfb76 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 26 Sep 2024 09:19:08 -0600 Subject: [PATCH 2/2] only skip asserts when __traverse__ is in play --- tests/test_gc.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/tests/test_gc.rs b/tests/test_gc.rs index f9f6d508746..1f55d91d496 100644 --- a/tests/test_gc.rs +++ b/tests/test_gc.rs @@ -37,11 +37,10 @@ fn class_with_freelist() { /// Tests that drop is eventually called on objects that are dropped when the /// GIL is not held. /// -/// /// On the free-threaded build, threads are resumed before tp_clear() calls -/// finish, so drop might not necessarily be called when a test checks. We -/// therefore cannot check that behavior in the free-threaded build without -/// introducing a flaky test +/// finish. Therefore, if the type needs __traverse__, drop might not necessarily +/// be called by the time the a test re-acquires a Python thread state and checks if +/// drop has been called. /// /// See https://peps.python.org/pep-0703/#stop-the-world struct TestDropCall { @@ -81,9 +80,7 @@ fn data_is_dropped() { drop(inst); }); - #[cfg(not(Py_GIL_DISABLED))] assert!(drop_called1.load(Ordering::Relaxed)); - #[cfg(not(Py_GIL_DISABLED))] assert!(drop_called2.load(Ordering::Relaxed)); } @@ -238,9 +235,7 @@ fn inheritance_with_new_methods_with_drop() { base.data = Some(Arc::clone(&drop_called2)); }); - #[cfg(not(Py_GIL_DISABLED))] assert!(drop_called1.load(Ordering::Relaxed)); - #[cfg(not(Py_GIL_DISABLED))] assert!(drop_called2.load(Ordering::Relaxed)); }