Skip to content

[python] chore: fix python 3.11 bug with async wait_for#488

Merged
leekeiabstraction merged 1 commit intoapache:mainfrom
fresh-borzoni:fix/python-311-asyncio-wait-for
Apr 10, 2026
Merged

[python] chore: fix python 3.11 bug with async wait_for#488
leekeiabstraction merged 1 commit intoapache:mainfrom
fresh-borzoni:fix/python-311-asyncio-wait-for

Conversation

@fresh-borzoni
Copy link
Copy Markdown
Contributor

@fresh-borzoni fresh-borzoni commented Apr 10, 2026

The tests wrapped async iterator consumption in asyncio.wait_for() as a safety net against hangs. But on Python prior to 3.12 fix(python/cpython#96764), wait_for itself has a bug (cpython#96764). Combined with future_into_py/Python::attach(), this creates a GIL deadlock. The safety net was causing the very hang it was meant to prevent.

pytest-timeout (30s per test) is the safety net now. It works at the process level, not the asyncio level, so it can't deadlock. If the async iterator hangs, it's a real bug that deserves a full stack trace from pytest-timeout.

related to https://github.com/apache/fluss-rust/actions/runs/24237394524/job/70768262474?pr=485

@fresh-borzoni
Copy link
Copy Markdown
Contributor Author

@leekeiabstraction PTAL 🙏
cc @qzyu999

@qzyu999
Copy link
Copy Markdown
Contributor

qzyu999 commented Apr 10, 2026

Hi @fresh-borzoni, great catch! I took a look and it appears that the entire functionality is preserved, while avoiding the downfall of the pre-3.12 deadlocking problem. Should this be noted somewhere in the documentation that users should avoid this Python function if they are running a Python version <3.12? We can add it in bindings/python/README.md or some other appropriate section:

Note on Asynchronous Iteration (Python < 3.12)
If you are consuming Fluss tables asynchronously in Python 3.11 or older, avoid wrapping the LogScanner async iterator directly in asyncio.wait_for(). Due to a known bug in CPython's task cancellation (Issue #96764), doing so can result in a GIL deadlock. If timeouts are necessary, it is highly recommended to upgrade to Python 3.12+, or handle timeouts at the process/test-runner level (e.g., using pytest-timeout).

Copy link
Copy Markdown
Contributor

@leekeiabstraction leekeiabstraction left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. Thank your for fixing!

@leekeiabstraction leekeiabstraction merged commit 6b997e6 into apache:main Apr 10, 2026
5 checks passed
@fresh-borzoni
Copy link
Copy Markdown
Contributor Author

fresh-borzoni commented Apr 10, 2026

@qzyu999 It's a good idea, thank you, but I prefer to wait a bit to verify that the fix is reliable and we don't have any further issues.
This fix provides us a proper termination with stack-trace, so it will easier to reason If something is still off after this fix.

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

Successfully merging this pull request may close these issues.

3 participants