Feedback from PR PyO3#975

src/once_cell.rs

@@ -31,9 +31,9 @@ impl<T> OnceCell<T> {
             return value;
         }
 
-        // Note that f() could release the GIL, so it's possible that another thread writes to this
-        // OnceCell before f() finishes. That's fine; we'll just have to discard the value
-        // computed here and accept a bit of wasted computation.
+        // Note that f() could temporarily release the GIL, so it's possible that another thread
+        // writes to this OnceCell before f() finishes. That's fine; we'll just have to discard
+        // the value computed here and accept a bit of wasted computation.
         let value = f();
         let _ = self.set(py, value);
 

src/type_object.rs

@@ -7,7 +7,6 @@ use crate::pyclass_init::PyObjectInit;
 use crate::types::{PyAny, PyType};
 use crate::{ffi, AsPyPointer, PyNativeType, Python};
 use parking_lot::{const_mutex, Mutex};
-use std::collections::HashSet;
 use std::thread::{self, ThreadId};
 
 /// `T: PyLayout<U>` represents that `T` is a concrete representaion of `U` in Python heap.
@@ -146,14 +145,14 @@ pub struct LazyStaticType {
     // Boxed because Python expects the type object to have a stable address.
     value: OnceCell<Box<ffi::PyTypeObject>>,
     // Threads which have begun initialization. Used for reentrant initialization detection.
-    initializing_threads: Mutex<Option<HashSet<ThreadId>>>,
+    initializing_threads: Mutex<Vec<ThreadId>>,
 }
 
 impl LazyStaticType {
     pub const fn new() -> Self {
         LazyStaticType {
             value: OnceCell::new(),
-            initializing_threads: const_mutex(None),
+            initializing_threads: const_mutex(Vec::new()),
         }
     }
 
@@ -167,14 +166,12 @@ impl LazyStaticType {
                     //
                     // That could lead to all sorts of unsafety such as using incomplete type objects
                     // to initialize class instances, so recursive initialization is prevented.
-                    let thread_not_already_initializing = self
-                        .initializing_threads
-                        .lock()
-                        .get_or_insert_with(HashSet::new)
-                        .insert(thread::current().id());
-
-                    if !thread_not_already_initializing {
+                    let thread_id = thread::current().id();
+                    let mut threads = self.initializing_threads.lock();
+                    if threads.contains(&thread_id) {
                         panic!("Recursive initialization of type_object for {}", T::NAME);
+                    } else {
+                        threads.push(thread_id)
                     }
                 }
 
@@ -190,7 +187,7 @@ impl LazyStaticType {
 
                 // Initialization successfully complete, can clear the thread list.
                 // (No futher calls to get_or_init() will try to init, on any thread.)
-                *self.initializing_threads.lock() = None;
+                *self.initializing_threads.lock() = Vec::new();
 
                 type_object
             })