-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
on_close called before open on short-lived WebSocket connections #2958
Comments
I've seen this in the wild too ... I think the |
This looks like an oversight - |
I made an attempt to fix this in #2966 |
TL;DR It's possible under the current implementation for
on_close
to be called beforeopen
for short-lived WebSocket connections. This is due to theon_close
callback being loaded before the connection is fully established.I am working on a project where we have a Python tornado-based WebSocket server that communicates with JavaScript WebSocket clients.
As part of our server class, we have overriden the functions
open
andon_close
ofwebsocket.WebSocketHandler
.In
open
we initialize some client-specific data and inon_close
we clean it up.Recently we have been dealing with a bug that was caused by
on_close
being called beforeopen
. This seems to happen if the client connection is unstable, and several attempts are made by the client before a WebSocket connection is established.When this happens, our code raises an error, as it is not able to cleanup a non initialized client.
We believe that the problem lies with how tornado accepts a new connection. It seems that the
on_close
callback is being loaded before the connection is fully established (andopen
is called), which means that there's a race condition in the open procedure for short lived connections.We were able to get the error every time by adding
time.sleep(1)
inself.accept_connection
:Has anyone else had this problem? Perhaps we are wrong to assume that
on_close
will never be called beforeopen
?The text was updated successfully, but these errors were encountered: