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

Don't break internal loops on CancelledError when the cancel is not triggered internally (avoid CTRL-C issue on python < 3.11) #642

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

diorcety
Copy link
Contributor

@diorcety diorcety commented Dec 13, 2024

Fix for #287
The issue comes from handling of KeyboardInterrupt before 3.11 (python/cpython@f08a191)
Before 3.11 all the tasks are cancelled when a KeyboardInterrupt is generated (https://github.com/python/cpython/blob/3.10/Lib/asyncio/runners.py#L47), leading that all internal loops are also cancelled, for example the client reader, flusher loops. Any call to nats client after a CTRL-PY will certainly timeout

In order to avoid such issue, this commit change the behaviour of CancelError inside the loop if this one is not due to internal cancel call.
The call to unsubscribe on PushSubscription is changed. Subscription loop is looking for is_closed from Subscription object, calling unsubscribe on super object of PushSubscription doesn't set correctly the flag _closed on the underlying Subscription.

I found during this work that the subscription inside _init_request_sub is never unsubscribed correctly.

@diorcety diorcety force-pushed the branch_fix_sigint branch 9 times, most recently from cc27968 to 4b41db5 Compare December 15, 2024 17:45
nats/aio/client.py Outdated Show resolved Hide resolved
nats/aio/client.py Outdated Show resolved Hide resolved
nats/aio/client.py Outdated Show resolved Hide resolved
…riggered internally (avoid CTRL-C issue on python < 3.11)
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.

2 participants