Open
Conversation
823ce5a to
15f6a6e
Compare
e21b021 to
55bbac6
Compare
515a793 to
696e680
Compare
This patch removes Python 3.7 from our supported versions list in the README. It also fixes a syntax error in the same list, causing version 3.12 to be rendered as 3.1220. Signed-off-by: Patrick M. Niedzielski <pniedzielski@bloomberg.net>
574759f to
c84e0bf
Compare
11d2f73 to
4d11aee
Compare
4d11aee to
22e3414
Compare
Signed-off-by: Patrick M. Niedzielski <pniedzielski@bloomberg.net>
22e3414 to
0ab55ef
Compare
The `on_message` callback, which is executed on one of `libbmq`’s internal processing threads, detected a potential deadlock by checking `sys.getrefcount(ext_session) == 2`, inferring that the callback held the last strong reference to the Cython `ExtSession`. If so, returning from the callback would trigger `__dealloc__`, which calls `stop()` on the C++ session synchronously. But, `stop()` waits for the callback’s own background thread to finish, causing a deadlock. However, according to the CPython developers, the return value of `sys.getrefcount` is not guaranteed to be any particular value, or to be stable across versions. In fact, Python 3.14 (particularly, with CPYthon commit 053c285f) optimized reference counting on the interpreter’s operand stack by skipping incrementing/decrementing when pushing/popping objects. This reduces the value that `sys.getrefcount()` returns in this situation by 1 compared with previous versions of Python. This causes the deadlock detector to trip spuriously in normal operation on Python 3.14. See python/cpython#132346 for details about this change and the position of the CPython developers. This patch replaces the refcount check with an explicit `owned_by_session` flag on the Cython `ExtSession` object. The Python `Session` sets the flag at construction while it holds a reference to the `ExtSession` and clears the flag in `__del__`. The `on_message` callback checks the flag, which answers the same question (“will returning from this callback cause `__dealloc__`?”) without depending on CPython refcount internals. Additionally, this patch introduces a `make_ext_session()` helper function for tests that construct the `ExtSession` directly, bypassing the Python `Session` wrapper. This helper sets the flag, mirroring what the real `Session` does, and avoids the deadlock detection tripping incorrectly in tests. Signed-off-by: Patrick M. Niedzielski <pniedzielski@bloomberg.net>
0ab55ef to
4f55c6d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request adds support to the BlazingMQ Python SDK for Python 3.14.
While usually version upgrades are easy, in this case, it was not. Python 3.14 made an undocumented change to the behavior of
sys.getrefcount, which we use to detect a potential deadlock when freeing the Cython extension's session object. Because of this, we cannot rely on the return value ofsys.getrefcount. Instead of changing what return value we expect depending on the Python interpreter version the code is running on, instead we add a flag to replace the deadlock detection. In the library itself, this change is simple, but it involves changing our tests to correctly handle this flag. Please see the commit message for the fully analysis.Closes: #59