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

Cancelling ping_server task potentially kills _conn_lost_cb execution. #43

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

Conversation

galindus
Copy link

Hi there,

We were trying to implement automatic reconnection to STAN when the connection was lost, and ran into an issue with the conn_lost_cb.

If you have a couple of awaitable, like calling stan_client.connect, the callback stops executing. This is due to, the whole task that is responsible for invoking conn_lost_cb (_ping_server_task) gets cancelled during the clean up of the client.

Someone with more experience in the code-base and more knowledge of asyncio could surely improve the change. But this is sufficient to be able to implement a retry mechanism.

The fix we have introduced is a shield around _close_due_to_ping_failure to allow the callback to full finish its execution.

The issue can easily be reproduced, by introducing an awaitable call inside conn_lost_cb, that is long enough for task to register it has been cancelled:

async def conn_lost_cb(err):
  await asyncio.sleep(0.1, loop=self.loop)
  print("This message is often not shown")

Notice that this does not only affect _conn_lost_cb, but potentially be any of the clean up work in _close_due_to_ping_failure after the _ping_server_task cancellation.

Cheers

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.

1 participant