Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for free-threaded Python #1241

Open
4 tasks
lysnikolaou opened this issue Jan 10, 2025 · 4 comments
Open
4 tasks

Support for free-threaded Python #1241

lysnikolaou opened this issue Jan 10, 2025 · 4 comments

Comments

@lysnikolaou
Copy link

Hi everyone! 👋

I'm opening an issue first of all to ask whether there's somebody that's already working on free-threading support. If not, I would like to help as much as I can. The steps are roughly the following:

  • Set up CI for free-threading (Not sure if this is relevant here, since AFAIU brotli does not exhaustively test different Python versions)
  • Audit C extension module for thread-safety issues
  • Mark C extension module as thread-safe with PyUnstable_Module_SetGIL
  • Build & upload nightly wheels for the free-threaded build (This includes some minor changes over at google/brotli-wheels, but those shouldn't be complicated at all)
@lysnikolaou
Copy link
Author

I've looked a bit deeper into this and it looks like there's no global state n the Python side that we need to lock around. However, it should be mentioned that Compressor, Decompressor instances cannot be shared across threads because of #501. There's two options we could go with here:

  • Explicitly state that instances of these two classes cannot be shared between threads and be done with it. In that case, marking the extension as free-threading compatible would be the only remaining thing.
  • Lock around calls to the C layer. We could use the PyObject-specific PyMutex for that and it should work okay. I can look into that if people think that's the best option.

@lysnikolaou
Copy link
Author

A third option that was pointed out to me by @ngoldbaum, which is what python-zstandard does as well, is to add an atomic flag to the Python-level object that signifies that the compressor/decompressor instance is in use by a thread, and then raise an error if another thread tries to use it as well.

@ngoldbaum
Copy link

which is what python-zstandard does as well

Not quite, there's only an open PR. We're still waiting to hear back from the python-zstandard maintainer. Which reminds me, I should give them a ping...

@lysnikolaou
Copy link
Author

Not quite, there's only an open PR.

Right! That's what I meant to say. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants