Don't define Py_LIMITED_API and don't re-enable GIL under free-threading#3482
Don't define Py_LIMITED_API and don't re-enable GIL under free-threading#3482bdarnell merged 6 commits intotornadoweb:masterfrom
Conversation
|
This needs a rebase on top of #3484 to pass CI. I previously found the build to work when Even though the module is thread-safe, I'm reluctant to call Because Tornado is a single-threaded framework, I'd prefer to avoid anything that implies future changes while free-threading is in its experimental phase. Minimizing churn here is more important to me than maximizing support for free-threading. (I'd be OK if the call to |
On macOS, I'm getting the following build error (which I find expected since the
Agreed. Since the |
|
Hmm, that error is clear and has been there since 3.13.0b1. Did I make a mistake in my earlier testing and not actually run a free-threaded build? The init changes still need some ifdefs because they use symbols introduced in python 3.12 and 3.13 but we support back to 3.9. |
Sorry for not making it clear in my last comment, but I had to use
Yup, missed that. On it. |
bdarnell
left a comment
There was a problem hiding this comment.
Ah yes, the TORNADO_EXTENSION option probably wasn't set right in some of my tests (but I did have it sometimes because I did have at least some failures).
This got more complicated than anticipated but I think it's almost ready to go.
tornado/speedups.c
Outdated
| #if (!defined(Py_LIMITED_API) && PY_VERSION_HEX >= 0x030c0000) || Py_LIMITED_API >= 0x030c0000 | ||
| {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED}, | ||
| #endif | ||
| #if !defined(Py_LIMITED_API) && PY_VERSION_HEX >= 0x030d0000 |
There was a problem hiding this comment.
Why are these two #ifs not the same structure? The definitions are the same. I think the second one also needs a Py_LIMITED_API >= 0x030d0000 clause.
It would be simpler to do #ifdef Py_mod_gil, although I don't think it's mandated that these are #defines instead of global variables.
There was a problem hiding this comment.
Yeah, I'd prefer to not go with #ifdef Py_mod_gil. These would probably not change, but it's still more stable to do a version check.
|
Also, there's only wheels for the stable ABI on PyPI. Would you be open to uploading wheels for |
|
I'm open to uploading cp313t wheels to pypi if someone else does the work ;) From my prior testing the blocker is that auditwheel was failing for unclear reasons so I assume the wheels aren't releasable until that's fixed (maybe just needs a version bump?) See tornado/.github/workflows/test.yml Line 152 in bfe7489 |
|
Anyway, this looks good so I'm going to go ahead and merge it. |
Py_LIMITED_APImacro leads to build failurestornado.speedupssince it's thread-safe.