[SDK] Python websockets client sometimes hangs on shutdown
The EventClient() in the Python SDK has a race condition in its shutdown. When
EventClient.close() is called, either:
- It shuts down successfully
- The event thread crashes and prints a stack trace (because this is in a daemon thread, it doesn't interfere with program shutdown)
Exception in thread WebSocketClient: Traceback (most recent call last): File "/usr/lib/python2.7/threading.py", line 810, in __bootstrap_inner self.run() File "/usr/lib/python2.7/threading.py", line 763, in run self.__target(*self.__args, **self.__kwargs) File "build/bdist.linux-x86_64/egg/ws4py/websocket.py", line 427, in run if not self.once(): File "build/bdist.linux-x86_64/egg/ws4py/websocket.py", line 300, in once b = self.sock.recv(self.reading_buffer_size) AttributeError: 'NoneType' object has no attribute 'recv'
- It hangs forever
The problem is:
- The parent class WebSocketClient.close() method is designed for orderly shutdown. It sends a close message to the server and prevents any more messages from being sent, but doesn't actually close the socket.
- The server is expected to respond with its own "closed" message.
- If the server is uncoorperative or stuck and doesn't respond with a "closed" message of its own, the client won't close the connection on its own. The server may even continue sending events, but if the application assumes that it doesn't receive any more events after returning from close(), it won't be prepared to handle them (because it is shutting down)
- To head this off, the current code calls close_connection() which explicitly closes the underlying socket. This also sets "WebSocketClient.sock" to "None". Unfortunately, as it turns out, the "threadedclient.WebSocketClient" is not threadsafe on this function. So this sometimes results in the above crash when sock is set to None at a bad time.
Proposed fix to EventClient.close():
- Call close() to start orderly shutdown
- Set a flag indicating that received_message() shouldn't forward any more messages to on_event(). Put a mutex in received_message() so that close() doesn't return until waits for any message handlers are completed.
- return from close(). At this point we don't care what happens because either the orderly shutdown will complete, or thread will be quietly killed and socket will get closed during process termination.
Hotfix: use a recursive lock for closed_lock so that EventClient.close() can be
called from on_event(). refs #7654