From 6402851107ed6dde885fc30d0d72b7b64addde09 Mon Sep 17 00:00:00 2001 From: "Michael J. Sullivan" Date: Mon, 26 Nov 2018 17:35:00 -0800 Subject: [PATCH] Fix a race condition in dmypy (#5956) While thinking about what might cause flakes in #5859, I found a race condition between `dmypy stop` and other `dmypy` commands. `dmypy stop` will send a response and close the socket before the status file has been deleted. This means that there is a window in which the daemon has reported to a client that it has exited but while a status file still exists. This can result in a number of issues, including the daemon appearing to be stuck (instead of stopped) to subsequent commands and also the exiting server deleting the status file of a subsequently started server. The fix is to remove the status file in `cmd_stop` before replying to the request. This ensures that, as far as clients are concerned, the daemon is exited after a stop command completes. I tested the bug and the fix by inserting a `time.sleep(1)` immediately before the `sys.exit(0)` in `serve`: this caused several tests to fail, and the changes fixed them. I believe that the observed flakes in the windows version in #5859 were caused by this issue, but in a way that the unix version was not susceptible to. --- mypy/dmypy_server.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 3bbcd84177fb..212cf7e5c440 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -153,6 +153,7 @@ def __init__(self, options: Options, def serve(self) -> None: """Serve requests, synchronously (no thread or fork).""" + command = None try: sock = self.create_listening_socket() if self.timeout is not None: @@ -200,7 +201,13 @@ def serve(self) -> None: reset_global_state() sys.exit(0) finally: - os.unlink(STATUS_FILE) + # If the final command is something other than a clean + # stop, remove the status file. (We can't just + # simplify the logic and always remove the file, since + # that could cause us to remove a future server's + # status file.) + if command != 'stop': + os.unlink(STATUS_FILE) finally: shutil.rmtree(self.sock_directory) exc_info = sys.exc_info() @@ -235,6 +242,11 @@ def cmd_status(self) -> Dict[str, object]: def cmd_stop(self) -> Dict[str, object]: """Stop daemon.""" + # We need to remove the status file *before* we complete the + # RPC. Otherwise a race condition exists where a subsequent + # command can see a status file from a dying server and think + # it is a live one. + os.unlink(STATUS_FILE) return {} def cmd_run(self, version: str, args: Sequence[str]) -> Dict[str, object]: