Expose unsafe wrappers for Py_BEGIN_CRITICAL_SECTION_MUTEX API

[ngoldbaum]

Python 3.14 introduced new variants for the critical section macros that accept a PyMutex instead of a PyObject. See python/cpython#133296, capi-workgroup/decisions#67, and python/cpython#135899 for more details.

I also was responsible for making sure this landed in 3.14, so shipping this PR is special to me as it completes the loop :)

So far I don't think there are any open source uses yet because it's so new and it's 3.14-specific.

The new APIs accept an EnteredCriticalSection which exposes unsafe get and get_mut methods because the caller needs to guarantee that the closure doesn't unsafely release the resource protected by the PyMutex.

I also expanded the docs for the existing critical section API wrappers to clarify things a bit.

[ngoldbaum]

I marked this ready for review but I expect it to have at least one more round. I left a couple questions for others and comments for myself inline.

[davidhewitt]

Suggested change

	/// a manner that would block. This is similar to how the GIL can be released
	/// a manner that would block. This is similar to how the GIL can be released

[davidhewitt]

Suggested change

	/// a manner that would block. This is similar to how the GIL can be released
	/// a manner that would block. This is similar to how the GIL can be released

[davidhewitt]

I would move this above the Safety section.

[davidhewitt]

We should probably state somewhere here that critical sections cannot be nested, and point from with_critical_section to with_critical_section2 if two locks / objects need to be locked simultaneously.

To reduce redundancy, I wonder if we should have a new module pyo3::sync::critical_sections, move all these APIs in there and have module-level documentation with a lot of this prose?

[davidhewitt]

I wonder, would the test be more interesting if we lock m1 and / or m2 here, and then below the second s.spawn call we add something like

// the other threads waiting for locks should not block this attach
Python::attach(|py| {
    // on free-threaded build, the critical sections should have blocked
    // the other threads from modification.
    #[cfg(Py_GIL_DISABLED)] 
    {
        assert_eq!(&*m1_guard, &[1, 2, 3]);
        assert_eq!(&*m2_guard, &[4, 5]);
    }

    drop(m1_guard);
    drop(m2_guard);
});

... so that way, the two threads both will wait until the locks are released? We should still be deadlock free, I believe.

Suggested change

	let m1_guard = m1.lock().unwrap();
	let m2_guard = m2.lock().unwrap();

[davidhewitt]

Might read a bit easier to do the comparisons against tuples:

Suggested change

	((*v1).eq(&expected1_vec1) && (*v2).eq(&expected1_vec2))
	|| ((*v1).eq(&expected2_vec1) && (*v2).eq(&expected2_vec2))
	(&*v1, &*v2) == (&expected1_vec1, &expected1_vec2))
	|| (&*v1, &*v2) == (&expected2_vec1, &expected2_vec2))

[davidhewitt]

I think this needs to be disconnected from the lifetime 'a

Suggested change

	F: FnOnce(EnteredCriticalSection<'a, T>) -> R,
	F: for<'section> FnOnce(EnteredCriticalSection<'section, T>) -> R,

... my reason is that otherwise it's possible to build code which "leaks" the section guard into the enclosing scope, as long as the borrow 'a on the mutex is longer:

let m = &mutex;
let mut outer_b = None;
with_critical_section_mutex(py, &mutex, |mut b| {
    // this assignment should be illegal
    outer_b = Some(b);
});
unsafe { outer_b.unwrap().get_mut() };

The good news is that you can then make the 'py and 'a lifetimes anonymous.

[ngoldbaum]

Nice! TIL about higher-ranked trait bounds.

[davidhewitt]

Similar to the above

Suggested change

	F: FnOnce(EnteredCriticalSection<'a, T1>, EnteredCriticalSection<'a, T2>) -> R,
	F: for<'section> FnOnce(EnteredCriticalSection<'section, T1>, EnteredCriticalSection<'section, T2>) -> R,

[ngoldbaum]

I think the last push satisfies all your comments and concerns.

[ngoldbaum]

I think tests are green now. I also added a release note for the moved functions.

[davidhewitt]

Looks great, just some very tiny nits. Thanks for this 🚀