pyo3 icon indicating copy to clipboard operation
pyo3 copied to clipboard

Add FFI definitions from `pythread.h`

Open LegionMammal978 opened this issue 11 months ago • 5 comments

This PR adds all definitions from pythread.h and cpython/pythread.h that haven't been deprecated or removed as of CPython 3.13. Of the functions, some are fully documented, some are undocumented but still contained in the limited API list, and some are neither documented nor in the limited API. Some of the limited-API functions, such as PyThread_get_thread_ident(), are widely used by 3rd-party code despite being undocumented, so it would be useful to include them in the bindings. The full breakdown:

  • Documented, limited API: functions PyThread_tss_alloc, PyThread_tss_create, PyThread_tss_delete, PyThread_tss_free, PyThread_tss_get, PyThread_tss_is_created, PyThread_tss_set; struct Py_tss_t.
  • Undocumented, limited API: functions PyThread_GetInfo, PyThread_acquire_lock, PyThread_acquire_lock_timed, PyThread_allocate_lock, PyThread_exit_thread, PyThread_free_lock, PyThread_get_stacksize, PyThread_get_thread_ident, PyThread_get_thread_native_id, PyThread_init_thread, PyThread_release_lock, PyThread_set_stacksize, PyThread_start_new_thread; enum PyLockStatus; typedefs PY_TIMEOUT_T, PyThread_type_lock; constants NOWAIT_LOCK, PY_LOCK_ACQUIRED, PY_LOCK_FAILURE, PY_LOCK_INTR, WAIT_LOCK.
  • Documented, non-limited API: constant Py_tss_NEEDS_INIT.
  • Undocumented, non-limited API: static PY_TIMEOUT_MAX; constant PYTHREAD_INVALID_THREAD_ID.

PyThread_get_thread_native_id() is a particularly fiddly definition due to all the OS-based #ifdefs guarding it: I've approximated them with cfg(target_os) values to the best of my ability, but it would be much easier if we could directly check that the PY_HAVE_THREAD_NATIVE_ID macro is defined. I've also gone through the PyPy and GraalPy versions of the headers to see which functions they support.

LegionMammal978 avatar Jan 24 '25 20:01 LegionMammal978

Hmm, I'm not entirely sure how to replicate the NATIVE_TSS_KEY_T without a billion cfg() guards. In CPython, it's the pthread_key_t provided by cpython/pthread_stubs.h:

#ifdef HAVE_PTHREAD_H
#   include <pthread.h>
#   define NATIVE_TSS_KEY_T     pthread_key_t
#elif defined(NT_THREADS)
#   define NATIVE_TSS_KEY_T     unsigned long
#elif defined(HAVE_PTHREAD_STUBS)
#   include "cpython/pthread_stubs.h"
#   define NATIVE_TSS_KEY_T     pthread_key_t
#else
#   error "Require native threads. See https://bugs.python.org/issue31370"
#endif

It might make sense just to add a carve-out for wasi here.

LegionMammal978 avatar Jan 24 '25 20:01 LegionMammal978

Alright, I've replaced PyThread_exit_thread() with a comment, and I've set NATIVE_TSS_KEY_T to c_uint on WASI to match the definition in cpython/pthread_stubs.h. It looks like CPython's configure script enables pthread_stubs.h only on WASI: on all other platforms without <pthread.h>, CPython will simply refuse to build. So adding a carve-out seems to be the correct option after all.

(As for following whatever libc does, it's unfortunately not nearly as simple as extracting a few cfgs. The crate has a whole tree of different platforms, each with its own set of type and function definitions, and replicating that would require a big, finicky list of explicitly-enabled platforms.)

LegionMammal978 avatar Apr 13 '25 23:04 LegionMammal978

I've set NATIVE_TSS_KEY_T to c_uint on WASI to match the definition in cpython/pthread_stubs.h. It looks like CPython's configure script enables pthread_stubs.h only on WASI: on all other platforms without <pthread.h>, CPython will simply refuse to build. So adding a carve-out seems to be the correct option after all.

That sounds reasonable, can you please add a comment about that?

It might make sense just to add a carve-out for wasi here.

That does seem the simplest. Can you sprinkle a bunch of #[cfg_attr(docsrs, doc(cfg(all())))] on that so the cfg banners in the docs aren't weird?

Anyway, I've been looking at the definition of PY_TIMEOUT_MAX and i'm not sure they're correct. The constants are different between versions. Also, all the cfg attributes will make them look weird in the docs.

I think we should try to mirror the python files as much as possible...I'm happy for ideas here.

/// private helper for `PY_TIMEOUT_MAX`
const LLONG_MAX: c_longlong  = ...;

/// Note: This is a static on 3.13 and up, thank the gods
#[cfg_attr(docsrs, doc(cfg(not(PyPy))))]
#[cfg(all(not(PyPy), not(Py_3_13)))]
pub const PY_TIMEOUT_MAX: c_longlong = if cfg!(Py_3_12) {
    if cfg!(not(windows)) {
        LLONG_MAX / 1000
    } else if cfg!(windows) {
        if 0xFFFFFFFELL.saturating_mul(1000) < LLONG_MAX {
            0xFFFFFFFELL.saturating_mul(1000)
        } else {
            LLONG_MAX
        }
    } else {
        LLONG_MAX
    }
} else if cfg!(Py_3_11) {

} else if cfg!(Py_3_10) {

} else if cfg!(Py_3_9) {

} else if cfg!(Py_3_8) {

} else {
    panic!()
};

There's also a definition for 3.7 which would need to live in src/pythread.rs or be re-exported. Or just not exist, I guess - I'm not sure when we're moving on from 3.7.

mejrs avatar Apr 14 '25 01:04 mejrs

That sounds reasonable, can you please add a comment about that?

Sure, will do, once I get the other stuff sorted out.

That does seem the simplest. Can you sprinkle a bunch of #[cfg_attr(docsrs, doc(cfg(all())))] on that so the cfg banners in the docs aren't weird?

Sorry, how exactly do these doc() attributes work? (I'm guessing it has something to do with doc_auto_cfg magic.) Why should we be using cfg(all())? And how do I test what it will look like?

Anyway, I've been looking at the definition of PY_TIMEOUT_MAX and i'm not sure they're correct. The constants are different between versions.

I copied the different versions as best I could. The change in CPython 3.11 (mirrored in GraalPy 24.1.0) is what these two separate lines are for:

#[cfg(all(not(PyPy), not(Py_3_11), windows))]
pub const PY_TIMEOUT_MAX: c_longlong = (0xFFFFFFFF as c_longlong).saturating_mul(1000);
#[cfg(all(not(PyPy), Py_3_11, not(Py_3_13), windows))]
pub const PY_TIMEOUT_MAX: c_longlong = (0xFFFFFFFE as c_longlong).saturating_mul(1000);

I use a saturating_mul() here because that's precisely the meaning of the #if/#else in the header. I can move it all into a const block, if the separate definitions look too wacky.

One lingering question: If neither _POSIX_THREADS nor NT_THREADS is defined, PY_TIMEOUT_MAX is set to LLONG_MAX as a fallback. The only such target is WASI on CPython 3.11+. On WASI, cpython/pthread_stubs.h defines _POSIX_THREADS, but PY_TIMEOUT_MAX is defined before that header is included. So on CPython 3.11 and 3.12, does WASI receive the _POSIX_THREADS LLONG_MAX / 1000, or the !_POSIX_THREADS LLONG_MAX? (On CPython 3.13+, the static value is set from a C file that indirectly includes cpython/pthread_stubs.h beforehand, so it should always be LLONG_MAX / 1000.) It may be a good idea to double-check what the value is on 3.11–3.13, if you have a working WASI setup.

There's also a definition for 3.7 which would need to live in src/pythread.rs or be re-exported. Or just not exist, I guess - I'm not sure when we're moving on from 3.7.

In fact, it existed in the limited src/pythread.h up until CPython 3.13, when it was removed from the limited API (python/cpython#110217). So I figured we should keep it out of abi3 on all Python versions, the same way we omit deprecated and removed functions. But I could change that (or just note it) if you'd like.

LegionMammal978 avatar Apr 14 '25 02:04 LegionMammal978

Reading these header files gives me a headache. Is this constant actually useful? I'm leaning towards just omitting everything but the static version.

Sorry, how exactly do these doc() attributes work

The problem is when you have something like

#[cfg(Py_3_10)]
pub const X = 10;

#[cfg(not(Py_3_10))]
pub const X = 5;

then if (say) the environment has a python 3.11 available and pyo3 detects that, it will be marked as "this is only available on 3.10 and up". doc_cfg lets you override that, and with doc(cfg(all())) (which is always true), that flag will not show up. Wrong banners can be confusing, see https://github.com/PyO3/pyo3/issues/2426 for example.

Or (in this case) you can do pub const X = if cfg!(Py_3_10) { 10} else {5};

And how do I test what it will look like?

See https://github.com/PyO3/pyo3/blob/main/Contributing.md#help-write-great-docs

I figured we should keep it out of abi3 on all Python versions

I agree, I read that PR as "it was never documented as being in the stable api", therefore it isn't.

mejrs avatar Apr 14 '25 09:04 mejrs