-
Notifications
You must be signed in to change notification settings - Fork 64
Add CFFI thread safety docs #188
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a global lock like this is necessary it is not safe for more than one thread to simultaneously call into any part of the library.
Missing "if" or "when" in that sentence?
As of version 2.0, CFFI generates thread-safe bindings to C libraries.
What's new in 2.0 is the GIL-free version of CFFI. CFFI has always generated thread-safe bindings with its GIL-using version, and now in 2.0 it continues to do so in its GIL-free version. I think that most of what you say in your (otherwise great and useful!) text applies equally to both versions.
You’re right, what’s there right now isn’t correct. I’ll fix it tomorrow. Thanks for giving this a read. |
Fixed, I also took the opportunity to expand the text a little with some more advice. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Co-authored-by: Matti Picus <[email protected]>
LGTM, will merge in a day or two unless another reviewer has more comments. |
ping @mattip - any change in plans? |
Thanks @ngoldbaum |
* Add CFFI thread safety docs * Delete incorrect statements * Add more links, examples, and suggestions about TSan * fix indentation in code example * Update doc/source/overview.rst Co-authored-by: Matti Picus <[email protected]> --------- Co-authored-by: Matti Picus <[email protected]> (cherry picked from commit e94a7b6)
* Explicitly specify --no-build-isolation as that's our expectation in these tests Previously we relied on pip to build the packages in non-PEP517 mode, which implied no build isolation. The latest `virtualenv` (with pypa/virtualenv#2868) won't include `wheel` in the virtualenv, which will mean that pip uses PEP-517 mode, which is isolated by default. (cherry picked from commit 24e42cb) * Add CFFI thread safety docs (#188) * Add CFFI thread safety docs * Delete incorrect statements * Add more links, examples, and suggestions about TSan * fix indentation in code example * Update doc/source/overview.rst Co-authored-by: Matti Picus <[email protected]> --------- Co-authored-by: Matti Picus <[email protected]> (cherry picked from commit e94a7b6) * Explicitly specify manylinux2014 in wheel building config (#184) (cherry picked from commit 078820c) * Enable more Windows pytest-run-parallel CI (#189) * enable windows pytest-run-parallel CI * pass skip-thread-unsafe * remove OS conditional (cherry picked from commit 3c61e14) * Misc CI env stabilization (#194) * Misc CI env stabilization * Specify explicit runner major image versions instead of `latest`. * Test only against versioned Python releases. Installing from arbitrary source commits with `-dev` is rarely worth the potential instability between runs. Specifying X.Y with `allow-prereleases: true` will use the latest packaged X.Y.Z release, falling back to the newest X.Y.0 pre-release if X.Y.0 has not yet been released. * correct manylinux image name typo (cherry picked from commit 51e276e) --------- Co-authored-by: Stefano Rivera <[email protected]> Co-authored-by: Nathan Goldbaum <[email protected]> Co-authored-by: Matti Picus <[email protected]>
c.f. #187
Adds new docs describing the thread safety guarantees CFFI offers. Includes a worked example demonstrating how to use a thread-unsafe C library safely with a
threading.Lock
.