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

No message on client's end when disconnecting #594

Closed
3 tasks done
flareglare opened this issue Dec 22, 2020 · 3 comments
Closed
3 tasks done

No message on client's end when disconnecting #594

flareglare opened this issue Dec 22, 2020 · 3 comments
Assignees
Labels

Comments

@flareglare
Copy link

What's the problem?

I wrote a simple client and server. Client sends platform data to server, and server sends a number of connected clients. When the client disconnects nothing is being printed out (client's disconnect() function seems to be not called). I just started learning this package so it might be just me not understanding some part of it.

What I've done so far

  • read the docs about event handlers
  • try to call sio.disconnect() function both from client.py and server.py
  • analysed this example

Talk is cheap, show me the code

Server

import eventlet
import socketio


sio = socketio.Server()
app = socketio.WSGIApp(sio)


global bg_task, clients
bg_task = None
clients = dict()


def background_task():
    print('Background task started!')
    while True:
        sio.sleep(3)
        sio.emit('clients_info', f'Connected clients: {len(clients)}')


@sio.event
def connect(sid, environ):
    global clients, bg_task

    print('Client connected\t', sid)
    clients.update({sid: environ})

    if not bg_task:
        sio.start_background_task(background_task)
        bg_task = True


@sio.event
def sys_data(sid, data):
    print(sid, data)


@sio.event
def disconnect(sid):
    try:
        sio.disconnect(sid)
        del clients[sid]
    except KeyError:
        print(f"Key {sid} was not found in client list")
    print('Client disconnected\t', sid)


if __name__ == '__main__':
    eventlet.wsgi.server(eventlet.listen(('', 6000)), app)

Client

import socketio
import platform
import time

sio = socketio.Client()


def background_task():
    # Stop the task if connection was dropped
    while sio.connected:
        # Send some platform data
        info = f'{platform.node()} {platform.system()}'
        sio.emit('sys_data', f'{time.asctime()} - sys_data - {info}')
        time.sleep(1)


@sio.event
def connect():
    print('Connection established')
    sio.start_background_task(background_task)


@sio.on('clients_info')
def my_response(data):
    print(data)


@sio.event
def disconnect():
    print('Disconnected from server')


sio.connect('http://localhost:6000/')
sio.sleep(5)
sio.disconnect()
@miguelgrinberg
Copy link
Owner

miguelgrinberg commented Dec 22, 2020

Yeah, there are a couple of problems in your code and you've also found a bug.

First of all, in the server you are calling disconnect() in the disconnect handler. That really serves no purpose. Your disconnect handler was invoked, that means that the client is already disconnecting.

The background task in your client is at risk of race conditions. Your while loop will end when the client is disconnected, but if the disconnect happens in the middle of a loop iteration the thread may end up making a Socket.IO call after the connection is gone. See the "How to Kill a Python Thread" article on my blog for ideas on how to stop the thread before your disconnect.

In the client you'd normally want to call sio.wait() at the very end, as this call will return when all the background tasks were stopped and is safe to exit. Many calls, including sio.disconnect() run in the background, so some additional time may be needed to stop everything before you exit the script.

The disconnect handler not being called is a bug in this package actually, so it is not your fault. The handler is invoked when the disconnect is initiated by the server, but it looks like I missed it when it is the client initiating. I'll fix that.

@miguelgrinberg miguelgrinberg self-assigned this Dec 22, 2020
@flareglare
Copy link
Author

Thanks for such detailed explanation of what's going on! 😮 I have few more questions Sir:

First of all, in the server you are calling disconnect() in the disconnect handler. That really serves no purpose

So the disconnect handler calls disconnect() "under the hood"?

The background task in your client is at risk of race conditions

Thanks for noticing. I haven't really touched threads so far in my learning. Would a try catch be enough to handle such case?

Many calls, including sio.disconnect() run in the background

So it's enough to call sio.wait() and when everything completes sio.disconnect() is run in the background without me having to call it, did I get it right?

@miguelgrinberg
Copy link
Owner

So the disconnect handler calls disconnect() "under the hood"?

No. The disconnect handler is invoked as a result of the client disconnecting. A client can disconnect on their own (i.e. closing the browser tab) or calling a disconnect() function in the client, or the server can call the disconnect() function to remove a client. In all these cases, your disconnect handler will be invoked, after the server is aware that the client is going away, so no need to call disconnect() again.

Would a try catch be enough to handle such case?

That's a bad solution that may or may not work, since due to different timings the error can be different every time. What exception are you going to catch? Catching all exceptions is a bad practice that can hide bugs in your applications, so that is not a good option. As I said before, I have a blog post on this topic, check that out.

So it's enough to call sio.wait() and when everything completes sio.disconnect() is run in the background

You have to call disconnect when you are done. If you call it from your background thread, the sio.wait() in the main thread will return when the disconnection is complete.

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