Actions
Bug #7654
closed[SDK] Python websockets client sometimes hangs on shutdown
Status:
Resolved
Priority:
Normal
Assigned To:
Category:
SDKs
Target version:
Story points:
0.5
Description
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.
Related issues
Updated by Peter Amstutz about 9 years ago
- Description updated (diff)
- Story points set to 1.0
Updated by Brett Smith about 9 years ago
- Category set to SDKs
- Assigned To set to Sarah Guthrie
- Target version set to 2015-11-11 sprint
Updated by Brett Smith about 9 years ago
- Target version changed from 2015-11-11 sprint to 2015-12-02 sprint
Updated by Peter Amstutz about 9 years ago
Fixed as described. Being a race condition, this is slippery to reproduce and even harder to test, so hopefully you agree with my diagnosis & fix.
Updated by Tom Clegg about 9 years ago
dec876f... even if there are other problems still lurking, it certainly looks better/safer now than before. LGTM.
Updated by Peter Amstutz about 9 years ago
- Status changed from New to Resolved
Applied in changeset arvados|commit:73a127e5492bc2711530b2f5a7c30a5021232d40.
Actions