Close TLS-based soundness holes by running closure on a newly created thread.

Cargo.toml

@@ -56,6 +56,7 @@ chrono-tz = ">= 0.6, < 0.10"
 trybuild = ">=1.0.70"
 proptest = { version = "1.0", default-features = false, features = ["std"] }
 send_wrapper = "0.6"
+scoped-tls-hkt = "0.1"
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0.61"
 rayon = "1.6.1"

examples/word-count/src/lib.rs

@@ -18,7 +18,7 @@ fn search_sequential(contents: &str, needle: &str) -> usize {
 
 #[pyfunction]
 fn search_sequential_allow_threads(py: Python<'_>, contents: &str, needle: &str) -> usize {
-    py.allow_threads(|| search_sequential(contents, needle))
+    py.allow_threads().with(|| search_sequential(contents, needle))
 }
 
 /// Count the occurrences of needle in line, case insensitive

guide/src/async-await.md

@@ -67,7 +67,7 @@ where
     fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
         let waker = cx.waker();
         Python::with_gil(|gil| {
-            gil.allow_threads(|| pin!(&mut self.0).poll(&mut Context::from_waker(waker)))
+            gil.allow_threads().with(|| pin!(&mut self.0).poll(&mut Context::from_waker(waker)))
         })
     }
 }

guide/src/features.md

@@ -107,10 +107,6 @@ Most users should only need a single `#[pymethods]` per `#[pyclass]`. In additio
 
 See [the `#[pyclass]` implementation details](class.md#implementation-details) for more information.
 
-### `nightly`
-
-The `nightly` feature needs the nightly Rust compiler. This allows PyO3 to use the `auto_traits` and `negative_impls` features to fix the `Python::allow_threads` function.
-
 ### `resolve-config`
 
 The `resolve-config` feature of the `pyo3-build-config` crate controls whether that crate's

guide/src/migration.md

@@ -135,6 +135,10 @@ Python::with_gil(|py| {
 ```
 </details>
 
+### `Python::allow_threads` was split into `Python::safe_allow_threads` and `Python::unsafe_allow_threads`
+
+TODO
+
 ### `Iter(A)NextOutput` are deprecated
 <details open>
 <summary><small>Click to expand</small></summary>

guide/src/parallelism.md

@@ -69,7 +69,7 @@ To enable parallel execution of this function, the [`Python::allow_threads`] met
 # }
 #[pyfunction]
 fn search_sequential_allow_threads(py: Python<'_>, contents: &str, needle: &str) -> usize {
-    py.allow_threads(|| search_sequential(contents, needle))
+    py.allow_threads().with(|| search_sequential(contents, needle))
 }
 ```
 

newsfragments/3646.changed.md

@@ -0,0 +1 @@
+`Python::allow_threads` now returns a builder-like object which allows choosing a strategy for running the closure, as the default one which now closes TLS-based soundness loopholes by dispatching the closure to a worker thread has significantly changed performance characteristics.

pyo3-benches/benches/bench_gil.rs

@@ -1,4 +1,5 @@
 use codspeed_criterion_compat::{criterion_group, criterion_main, Bencher, Criterion};
+use std::hint::black_box;
 
 use pyo3::prelude::*;
 
@@ -13,9 +14,24 @@ fn bench_dirty_acquire_gil(b: &mut Bencher<'_>) {
     b.iter(|| Python::with_gil(|py| obj.clone_ref(py)));
 }
 
+fn bench_allow_threads(b: &mut Bencher<'_>) {
+    Python::with_gil(|py| {
+        py.allow_threads().with(|| ());
+        b.iter(|| py.allow_threads().with(|| black_box(42)));
+    });
+}
+
+fn bench_local_allow_threads(b: &mut Bencher<'_>) {
+    Python::with_gil(|py| {
+        b.iter(|| unsafe { py.allow_threads().local() }.with(|| black_box(42)));
+    });
+}
+
 fn criterion_benchmark(c: &mut Criterion) {
     c.bench_function("clean_acquire_gil", bench_clean_acquire_gil);
     c.bench_function("dirty_acquire_gil", bench_dirty_acquire_gil);
+    c.bench_function("allow_threads", bench_allow_threads);
+    c.bench_function("local_allow_threads", bench_local_allow_threads);
 }
 
 criterion_group!(benches, criterion_benchmark);

src/err/mod.rs

@@ -39,8 +39,6 @@ pub struct PyErr {
 }
 
 // The inner value is only accessed through ways that require proving the gil is held
-#[cfg(feature = "nightly")]
-unsafe impl crate::marker::Ungil for PyErr {}
 unsafe impl Send for PyErr {}
 unsafe impl Sync for PyErr {}
 

src/gil.rs

@@ -735,23 +735,25 @@ mod tests {
 
     #[test]
     fn test_allow_threads() {
-        assert!(!gil_is_acquired());
+        for _ in 0..10 {
+            assert!(!gil_is_acquired());
 
-        Python::with_gil(|py| {
-            assert!(gil_is_acquired());
+            Python::with_gil(|py| {
+                assert!(gil_is_acquired());
 
-            py.allow_threads(move || {
-                assert!(!gil_is_acquired());
+                py.allow_threads().with(move || {
+                    assert!(!gil_is_acquired());
 
-                Python::with_gil(|_| assert!(gil_is_acquired()));
+                    Python::with_gil(|_| assert!(gil_is_acquired()));
 
-                assert!(!gil_is_acquired());
-            });
+                    assert!(!gil_is_acquired());
+                });
 
-            assert!(gil_is_acquired());
-        });
+                assert!(gil_is_acquired());
+            });
 
-        assert!(!gil_is_acquired());
+            assert!(!gil_is_acquired());
+        }
     }
 
     #[cfg(feature = "py-clone")]
@@ -763,7 +765,7 @@ mod tests {
             let obj = get_object(py);
             assert!(obj.get_refcnt(py) == 1);
             // Clone the object without the GIL which should panic
-            py.allow_threads(|| obj.clone());
+            py.allow_threads().with(|| obj.clone());
         });
     }
 

src/instance.rs

@@ -922,8 +922,6 @@ impl<T> IntoPy<PyObject> for Borrowed<'_, '_, T> {
 pub struct Py<T>(NonNull<ffi::PyObject>, PhantomData<T>);
 
 // The inner value is only accessed through ways that require proving the gil is held
-#[cfg(feature = "nightly")]
-unsafe impl<T> crate::marker::Ungil for Py<T> {}
 unsafe impl<T> Send for Py<T> {}
 unsafe impl<T> Sync for Py<T> {}
 

src/lib.rs

@@ -1,5 +1,4 @@
 #![warn(missing_docs)]
-#![cfg_attr(feature = "nightly", feature(auto_traits, negative_impls))]
 #![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
 // Deny some lints in doctests.
 // Use `#[allow(...)]` locally to override.
@@ -115,10 +114,6 @@
 //! [`Py`]`<T>` for all `T` that implement [`Serialize`] and [`Deserialize`].
 //! - [`smallvec`][smallvec]: Enables conversions between Python list and [smallvec]'s [`SmallVec`].
 //!
-//! ## Unstable features
-//!
-//! - `nightly`: Uses  `#![feature(auto_traits, negative_impls)]` to define [`Ungil`] as an auto trait.
-//
 //! ## `rustc` environment flags
 //!
 //! PyO3 uses `rustc`'s `--cfg` flags to enable or disable code used for different Python versions.
@@ -314,7 +309,6 @@
 //! [Python from Rust]: https://github.com/PyO3/pyo3#using-python-from-rust
 //! [Rust from Python]: https://github.com/PyO3/pyo3#using-rust-from-python
 //! [Features chapter of the guide]: https://pyo3.rs/latest/features.html#features-reference "Features Reference - PyO3 user guide"
-//! [`Ungil`]: crate::marker::Ungil
 pub use crate::class::*;
 pub use crate::conversion::{AsPyPointer, FromPyObject, IntoPy, ToPyObject};
 #[cfg(feature = "gil-refs")]