miguelgrinberg / python-socketio

Python Socket.IO server and client
MIT License
3.96k stars 587 forks source link

The library sets signal handler in a way that disables signals handlers previously set by application with `asyncio.loop.add_signal_handler` #1390

Open oliora opened 5 days ago

oliora commented 5 days ago

python-socketio library sets its own signal handler in a way that disables signals handlers that were previously set with asyncio.loop.add_signal_handler

To Reproduce

import asyncio
import signal
import socketio

URL = ...  # SocketsIO server URL 'ws://...'

def signal_handler(sig: signal.Signals):
    print(f'Received signal {sig}. Stopping...')
    raise KeyboardInterrupt()

async def run():
    loop = asyncio.get_event_loop()
    for sig in (signal.SIGINT, signal.SIGTERM):
        loop.add_signal_handler(sig, signal_handler, sig)

    async with socketio.AsyncSimpleClient() as ws:
        await ws.connect(URL, transports=['websocket'])
        print('Waiting...')
        await asyncio.sleep(10000000000000)

asyncio.run(run())

Steps:

  1. On Linux system run the above program
  2. Wait until Waiting... is printed
  3. Press Ctrl+C
  4. Application is stopped without printing Received signal ...

Expected behavior

  1. (After Ctrl+C is pressed) Received signal ... is printed then application is stopped.

If I comment line await ws.connect(....) then the application behavior is as expected.

Additional context Aside from broken applications it is just a bad practice if library messes up with such a global thing as signal handlers. Signal handlers should be left to the application concern.

miguelgrinberg commented 5 days ago

If you want to handle sigint yourself you have to disable the handling in this client:

async with socketio.AsyncSimpleClient(handle_sigint=False) as ws:
   # ...

To my knowledge there is no way to have two sigint handlers installed in the same loop.

For any other signals outside of SIGINT there shouldn't be a problem, this library only provides a default SIGINT handler.

oliora commented 5 days ago

What are the consequences of not handling signals in the client?

miguelgrinberg commented 5 days ago

All the default handler does is disconnect any active clients gracefully. If you use your own you will want to do this yourself.

oliora commented 5 days ago

Thank you. Do I get the same effect if I use [async] with client?

miguelgrinberg commented 5 days ago

Ah yes, with the simple client it should when it exits the context manager block.