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

Unable to cancel asynchronous task while client is reconnecting #1333

Closed
maxhlc opened this issue May 3, 2024 · 3 comments
Closed

Unable to cancel asynchronous task while client is reconnecting #1333

maxhlc opened this issue May 3, 2024 · 3 comments
Labels

Comments

@maxhlc
Copy link

maxhlc commented May 3, 2024

IMPORTANT: If you have a question, or you are not sure if you have found a bug in this package, then you are in the wrong place. Hit back in your web browser, and then open a GitHub Discussion instead. Likewise, if you are unable to provide the information requested below, open a discussion to troubleshoot your issue.

Describe the bug

I am using the library to connect to multiple Socket.IO servers at the same time within a single process using the asynchronous version of the client. The code is designed to reconnect to these servers automatically as they are not guaranteed to be online throughout the entire lifetime of the process. I pass the retry=True flag during the initial call to AsyncClient.connect so that each client continues to attempt to reconnect.

The asynchronous clients are initialised with handle_sigint=False so that I can handle signals myself. When SIGINT is caught by my code, I am cancelling the asyncio tasks corresponding to the various clients. I have noticed that attempting to kill an AsyncClient task fails if it is within its reconnection loop. The following behaviour does not affect clients that are connected when the corresponding asyncio task is cancelled.

I believe that a try-catch statement in the AsyncClient._handle_reconnect method is causing the issue. This statement, within the reconnection loop at lines 470-488, ignores the asyncio.CancelledError exception raised when cancelling a task. The task never stops, therefore the process cannot finish without forcefully stopping the process outright with SIGKILL.

When I remove asyncio.CancelledError from the list of ignored exceptions, the task cancels successfully, allowing me to stop and close the event loop and exit the process.

I would be interested to see if this behaviour is reproducible by others or whether my implemention is wrong and the try-except block is trying to guard against a different issue.

To Reproduce
Steps to reproduce the behavior:

  1. Create an AsyncClient
  2. Try to connect to an unavailable Socket.IO instance with connection retry enabled
  3. Try to cancel the asynchronous task of the client
  4. The asyncio.CancelledError exception is ignored and the client continues to attempt reconnection

Below is a small example to reproduce the issue. It attempts to stop the client after 5 seconds and will fail without any modification to the reconnection loop code.

NOTE: this code will not stop by itself. You may need to send SIGKILL manually to stop it.

# Standard imports
import asyncio

# Third-party imports
from socketio import AsyncClient


async def kill():
    # Print sleep message
    print("Sleeping for 5 seconds before attempting to stop")

    # Wait for some time before cancelling all tasks
    await asyncio.sleep(5)

    # Print attempt message
    print("Attempting to cancel tasks")

    # Get tasks to cancel
    tasks = [task for task in asyncio.all_tasks() if task is not asyncio.current_task()]

    # Cancel tasks
    [task.cancel() for task in tasks]

    # Wait for tasks to finish
    await asyncio.gather(*tasks, return_exceptions=True)

    # Print cancellation message
    print("Tasks cancelled")

    # Stop loop
    loop.stop()


async def main():
    # Declare URL for nonexistent Socket.IO server
    url = "localhost:1234"

    # Declare client
    client = AsyncClient()

    # Try to connect
    await client.connect(url, retry=True)


if __name__ == "__main__":
    # Get event loop
    loop = asyncio.get_event_loop()

    # Add main task
    mainTask = loop.create_task(main())

    # Add kill task
    killTask = loop.create_task(kill())

    try:
        # Keep running loop
        loop.run_forever()
    finally:
        # Close loop
        loop.close()

Expected behavior
During the reconnection loop, asyncio.CancelledError exceptions should not be ignored so that the task can be cancelled when the client is attempting to reconnect.

Logs
N/A

Additional context
N/A

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented May 3, 2024

This is similar to #1277. It is a known limitation right now that the reconnection loop does not have a public way to be aborted. I intend to offer a cancellation option.

I wrote this code several years ago and honestly do not remember why I'm catching CancelledError in addition to TimeoutError. Looking at it now I agree that it makes more sense to handle CancelledError in the same way as an abort.

@maxhlc
Copy link
Author

maxhlc commented May 4, 2024

Glad to hear that it's on the roadmap!

When I came across the try-catch block, my first thought was that it might have been leftover from some edge case during development.

@miguelgrinberg
Copy link
Owner

The new shutdown() method disconnects a client if it is connected, or stops a reconnect loop if it is trying to reconnect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants