From 63aa4e22e4ddfb2def2927480e9520e375df3291 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Thu, 25 Oct 2018 21:07:18 -0700 Subject: [PATCH 01/50] Initial dmypy for Windows work --- mypy/dmypy.py | 88 +++++++++++++++++---- mypy/dmypy_server.py | 178 ++++++++++++++++++++++++++++++++----------- mypy/dmypy_util.py | 27 +++++-- 3 files changed, 226 insertions(+), 67 deletions(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index 0ce868aef834..c84f14fcb6ff 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -9,11 +9,15 @@ import argparse import json import os +import pickle import signal import socket import sys import time +# This may be private, but it is needed for IPC on Windows, and is basically stable +import _winapi + from typing import Any, Callable, Dict, List, Mapping, Optional, Tuple from mypy.dmypy_util import STATUS_FILE, receive @@ -95,7 +99,7 @@ def __init__(self, prog: str) -> None: help="Server shutdown timeout (in seconds)") p.add_argument('flags', metavar='FLAG', nargs='*', type=str, help="Regular mypy flags (precede with --)") - +p.add_argument('--options-file', help="For internal use only") help_parser = p = subparsers.add_parser('help') del p @@ -183,10 +187,14 @@ def start_server(args: argparse.Namespace, allow_sources: bool = False) -> None: """Start the server from command arguments and wait for it.""" # Lazy import so this import doesn't slow down other commands. from mypy.dmypy_server import daemonize, Server, process_start_options - if daemonize(Server(process_start_options(args.flags, allow_sources), - timeout=args.timeout).serve, - args.log_file) != 0: - sys.exit(1) + start_options = process_start_options(args.flags, allow_sources) + if sys.platform == 'win32': + daemonize(start_options, timeout=args.timeout) + else: + if daemonize(Server(start_options, + timeout=args.timeout).serve, + args.log_file) != 0: + sys.exit(1) wait_for_server() @@ -278,7 +286,11 @@ def do_kill(args: argparse.Namespace) -> None: """Kill daemon process with SIGKILL.""" pid, sockname = get_status() try: - os.kill(pid, signal.SIGKILL) + if sys.platform == 'win32': + sig = 1 # On Windows, kill is always sigkill, so this is the exit code + else: + sig = signal.SIGKILL + os.kill(pid, sig) except OSError as err: sys.exit(str(err)) else: @@ -364,7 +376,14 @@ def do_daemon(args: argparse.Namespace) -> None: """Serve requests in the foreground.""" # Lazy import so this import doesn't slow down other commands. from mypy.dmypy_server import Server, process_start_options - Server(process_start_options(args.flags, allow_sources=False), timeout=args.timeout).serve() + if args.options_file: + with open(args.options_file) as f: + options, timeout = pickle.load(f) + else: + options = process_start_options(args.flags, allow_sources=False) + timeout = args.timeout + sys.exit(0) # FIXME: this is just for testing + Server(options, timeout=timeout).serve() @action(help_parser) @@ -392,22 +411,59 @@ def request(command: str, *, timeout: Optional[float] = None, args = dict(kwds) args.update(command=command) bdata = json.dumps(args).encode('utf8') - pid, sockname = get_status() - sock = socket.socket(socket.AF_UNIX) - if timeout is not None: - sock.settimeout(timeout) + pid, name = get_status() try: - sock.connect(sockname) - sock.sendall(bdata) - sock.shutdown(socket.SHUT_WR) - response = receive(sock) + if sys.platform == 'win32': + # Sadly we need to poll until this works, or it times out + # thanks Windows + while True: + try: + handle = _winapi.CreateFile( + name, + _winapi.GENERIC_READ | _winapi.GENERIC_WRITE, + 0, + _winapi.NULL, + _winapi.OPEN_EXISTING, + 0, + _winapi.NULL, + ) + _winapi.SetNamedPipeHandleState(handle, + _winapi.NULL, + _winapi.NULL, + _winapi.NULL) + except FileNotFoundError: + raise WindowsError( + "Unable to open connection to pipe at {}".format(name) + ) + except WindowsError as e: + if e.winerror not in (_winapi.ERROR_SEM_TIMEOUT, _winapi.ERROR_PIPE_BUSY): + return {'error'} + else: + time.sleep(1) + break + # great, we have a named pipe connection! Now write our data + _winapi.WriteFile(handle, bdata, len(bdata)) + # now get the response + response = receive(handle) + + else: + sock = socket.socket(socket.AF_UNIX) + if timeout is not None: + sock.settimeout(timeout) + sock.connect(name) + sock.sendall(bdata) + sock.shutdown(socket.SHUT_WR) + response = receive(sock) except OSError as err: return {'error': str(err)} # TODO: Other errors, e.g. ValueError, UnicodeError else: return response finally: - sock.close() + if sys.platform == 'win32': + _winapi.CloseHandle(handle) + else: + sock.close() def get_status() -> Tuple[int, str]: diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 9e05818ee62e..0934627fbf3c 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -6,15 +6,20 @@ to enable fine-grained incremental reprocessing of changes. """ +import argparse import json import os +import pickle import shutil import socket +import subprocess import sys import tempfile import time import traceback +import _winapi + from typing import AbstractSet, Any, Callable, Dict, List, Mapping, Optional, Sequence, Set, Tuple import mypy.build @@ -38,53 +43,72 @@ MEM_PROFILE = False # type: Final # If True, dump memory profile after initialization -def daemonize(func: Callable[[], None], log_file: Optional[str] = None) -> int: - """Arrange to call func() in a grandchild of the current process. +DETACHED_PROCESS = 0x00000008 - Return 0 for success, exit status for failure, negative if - subprocess killed by signal. - """ - # See https://stackoverflow.com/questions/473620/how-do-you-create-a-daemon-in-python - # mypyc doesn't like unreachable code, so trick mypy into thinking the branch is reachable - if sys.platform == 'win32' or bool(): - raise ValueError('Mypy daemon is not supported on Windows yet') - sys.stdout.flush() - sys.stderr.flush() - pid = os.fork() - if pid: - # Parent process: wait for child in case things go bad there. - npid, sts = os.waitpid(pid, 0) - sig = sts & 0xff - if sig: - print("Child killed by signal", sig) - return -sig - sts = sts >> 8 - if sts: - print("Child exit status", sts) - return sts - # Child process: do a bunch of UNIX stuff and then fork a grandchild. - try: - os.setsid() # Detach controlling terminal - os.umask(0o27) - devnull = os.open('/dev/null', os.O_RDWR) - os.dup2(devnull, 0) - os.dup2(devnull, 1) - os.dup2(devnull, 2) - os.close(devnull) + +if sys.platform == 'win32': + def daemonize(opts: Options, timeout: Optional[int] = None) -> int: + """Create the daemon process via "dmypy daemon" and pass options via file + + This uses the DETACHED_PROCESS flag to invoke the Server. + See https://docs.microsoft.com/en-us/windows/desktop/procthread/process-creation-flags + + It also pickles the options to be unpickled by mypy. + """ + command = [sys.executable, '-m', 'mypy.dmypy', 'daemon'] + with tempfile.TemporaryDirectory() as dir: + options_file = os.path.join(dir, 'dmypy_options.pickle') + with open(options_file, 'wb') as file: + pickle.dump((opts, timeout), file) + command.append('--options-file={}'.format(options_file)) + print(command) + subprocess.run(command, creationflags=DETACHED_PROCESS) + +else: + def daemonize(func: Callable[[], None], log_file: Optional[str] = None) -> int: + """Arrange to call func() in a grandchild of the current process. + + Return 0 for success, exit status for failure, negative if + subprocess killed by signal. + """ + # See https://stackoverflow.com/questions/473620/how-do-you-create-a-daemon-in-python + sys.stdout.flush() + sys.stderr.flush() pid = os.fork() if pid: - # Child is done, exit to parent. - os._exit(0) - # Grandchild: run the server. - if log_file: - sys.stdout = sys.stderr = open(log_file, 'a', buffering=1) - fd = sys.stdout.fileno() - os.dup2(fd, 2) - os.dup2(fd, 1) - func() - finally: - # Make sure we never get back into the caller. - os._exit(1) + # Parent process: wait for child in case things go bad there. + npid, sts = os.waitpid(pid, 0) + sig = sts & 0xff + if sig: + print("Child killed by signal", sig) + return -sig + sts = sts >> 8 + if sts: + print("Child exit status", sts) + return sts + # Child process: do a bunch of UNIX stuff and then fork a grandchild. + try: + os.setsid() # Detach controlling terminal + os.umask(0o27) + devnull = os.open('/dev/null', os.O_RDWR) + os.dup2(devnull, 0) + os.dup2(devnull, 1) + os.dup2(devnull, 2) + os.close(devnull) + pid = os.fork() + if pid: + # Child is done, exit to parent. + os._exit(0) + # Grandchild: run the server. + if log_file: + sys.stdout = sys.stderr = open(log_file, 'a', buffering=1) + fd = sys.stdout.fileno() + os.dup2(fd, 2) + os.dup2(fd, 1) + func() + finally: + # Make sure we never get back into the caller. + os._exit(1) # Server code. @@ -124,6 +148,8 @@ class Server: # NOTE: the instance is constructed in the parent process but # serve() is called in the grandchild (by daemonize()). + buffer_size = 2**16 + def __init__(self, options: Options, timeout: Optional[int] = None) -> None: """Initialize the server with the desired mypy flags.""" @@ -153,6 +179,70 @@ def __init__(self, options: Options, def serve(self) -> None: """Serve requests, synchronously (no thread or fork).""" + if sys.platform == 'win32': + self.serve_win() + else: + self.serve_nix() + + def serve_win(self) -> None: + name = r'\\.\pipe\{}'.format(SOCKET_NAME) + try: + with open(STATUS_FILE, 'w') as f: + json.dump({'pid': os.getpid(), 'sockname': name}, f) + f.write('\n') # I like my JSON with trailing newline + while True: + handle = _winapi.CreateNamedPipe(name, + _winapi.PIPE_ACCESS_DUPLEX | _winapi.FILE_FLAG_FIRST_PIPE_INSTANCE, + _winapi.PIPE_READMODE_MESSAGE | _winapi.PIPE_TYPE_MESSAGE | _winapi.PIPE_WAIT, + 1, # one instance + self.buffer_size, + self.buffer_size, + 0, # PIPE_NOWAIT + 0, # PIPE_ACCEPT_REMOTE_CLIENTS (this is the default, so I guess?) + ) + if _winapi.GetLastError() != 0: + err = _winapi.GetLastError() + print('Error creating pipe: {err}'.format(err)) + continue + _winapi.ConnectNamedPipe(handle, _winapi.NULL) + + if handle != _winapi.NULL: + while True: + try: + data = receive(handle) + except OSError: + continue + + resp = {} # type: Dict[str, Any] + if 'command' not in data: + resp = {'error': "No command found in request"} + else: + command = data['command'] + if not isinstance(command, str): + resp = {'error': "Command is not a string"} + else: + command = data.pop('command') + try: + resp = self.run_command(command, data) + except Exception: + # If we are crashing, report the crash to the client + tb = traceback.format_exception(*sys.exc_info()) + resp = {'error': "Daemon crashed!\n" + "".join(tb)} + _winapi.WriteFile(json.dumps(resp).encode('utf8')) + raise + try: + _winapi.WriteFile(json.dumps(resp).encode('utf8')) + except OSError: + pass # Maybe the client hung up + + if command == 'stop': + _winapi.CloseHandle(handle) + reset_global_state() + sys.exit(0) + finally: + os.unlink(STATUS_FILE) + + def serve_nix(self) -> None: try: sock = self.create_listening_socket() if self.timeout is not None: diff --git a/mypy/dmypy_util.py b/mypy/dmypy_util.py index d5aacfd1dc45..9d6080a4c84e 100644 --- a/mypy/dmypy_util.py +++ b/mypy/dmypy_util.py @@ -5,8 +5,11 @@ import json import socket +import sys -from typing import Any +import _winapi + +from typing import Any, Union MYPY = False if MYPY: @@ -14,8 +17,10 @@ STATUS_FILE = '.dmypy.json' # type: Final +HANDLE = int + -def receive(sock: socket.socket) -> Any: +def receive(connection: Union[socket.socket, HANDLE]) -> Any: """Receive JSON data from a socket until EOF. Raise a subclass of OSError if there's a socket exception. @@ -24,11 +29,19 @@ def receive(sock: socket.socket) -> Any: not a dict. """ bdata = bytearray() - while True: - more = sock.recv(100000) - if not more: - break - bdata.extend(more) + read_size = 100000 + if isinstance(connection, HANDLE): + while True: + more = _winapi.ReadFile(connection, read_size) + if not more: + break + bdata.extend(more) + else: + while True: + more = connection.recv(read_size) + if not more: + break + bdata.extend(more) if not bdata: raise OSError("No data received") try: From abc64b7713a4fb926c916434444536e102229cb1 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Tue, 30 Oct 2018 16:00:27 -0700 Subject: [PATCH 02/50] Fixes related to dmypy server on Windows --- mypy/dmypy_server.py | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 0934627fbf3c..f7099b3c5ae3 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -185,7 +185,8 @@ def serve(self) -> None: self.serve_nix() def serve_win(self) -> None: - name = r'\\.\pipe\{}'.format(SOCKET_NAME) + name = r'\\.\pipe\python-{}-{}'.format(os.getpid(), SOCKET_NAME) + handle = _winapi.NULL try: with open(STATUS_FILE, 'w') as f: json.dump({'pid': os.getpid(), 'sockname': name}, f) @@ -197,22 +198,21 @@ def serve_win(self) -> None: 1, # one instance self.buffer_size, self.buffer_size, - 0, # PIPE_NOWAIT + 0, # PIPE_WAIT, wait for a connection of the client 0, # PIPE_ACCEPT_REMOTE_CLIENTS (this is the default, so I guess?) ) - if _winapi.GetLastError() != 0: + if handle == -1: # INVALID_HANDLE_VALUE err = _winapi.GetLastError() - print('Error creating pipe: {err}'.format(err)) - continue - _winapi.ConnectNamedPipe(handle, _winapi.NULL) - - if handle != _winapi.NULL: - while True: + print('Invalid handle to pipe: {err}'.format(err)) + sys.exit(1) + res = _winapi.ConnectNamedPipe(handle, _winapi.NULL) + # we can connect if we connected, or the client already connected + if res or _winapi.GetLastError() == _winapi.ERROR_PIPE_CONNECTED: + while True: try: data = receive(handle) except OSError: continue - resp = {} # type: Dict[str, Any] if 'command' not in data: resp = {'error': "No command found in request"} @@ -228,10 +228,10 @@ def serve_win(self) -> None: # If we are crashing, report the crash to the client tb = traceback.format_exception(*sys.exc_info()) resp = {'error': "Daemon crashed!\n" + "".join(tb)} - _winapi.WriteFile(json.dumps(resp).encode('utf8')) + _winapi.WriteFile(handle, json.dumps(resp).encode('utf8')) raise try: - _winapi.WriteFile(json.dumps(resp).encode('utf8')) + _winapi.WriteFile(handle, json.dumps(resp).encode('utf8')) except OSError: pass # Maybe the client hung up @@ -241,6 +241,8 @@ def serve_win(self) -> None: sys.exit(0) finally: os.unlink(STATUS_FILE) + if handle != _winapi.NULL: + _winapi.CloseHandle(handle) def serve_nix(self) -> None: try: From cf33bfa488a51538b9af140194c1b5838a913aa8 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Tue, 30 Oct 2018 22:18:20 -0700 Subject: [PATCH 03/50] Get dmypy start,stop,run,kill,check,restart working! :tada: --- mypy/dmypy.py | 89 ++++++++++++++++++++++++++++---------------- mypy/dmypy_server.py | 86 +++++++++++++++++++++++------------------- mypy/dmypy_util.py | 2 +- 3 files changed, 104 insertions(+), 73 deletions(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index c84f14fcb6ff..47777f58f02a 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -12,11 +12,18 @@ import pickle import signal import socket +import subprocess import sys import time -# This may be private, but it is needed for IPC on Windows, and is basically stable -import _winapi +if sys.platform == 'win32': + # This may be private, but it is needed for IPC on Windows, and is basically stable + import _winapi + import ctypes + from ctypes import wintypes + + OpenProcess = ctypes.windll.kernel32.OpenProcess + GetExitCodeProcess = ctypes.windll.kernel32.GetExitCodeProcess from typing import Any, Callable, Dict, List, Mapping, Optional, Tuple @@ -189,7 +196,8 @@ def start_server(args: argparse.Namespace, allow_sources: bool = False) -> None: from mypy.dmypy_server import daemonize, Server, process_start_options start_options = process_start_options(args.flags, allow_sources) if sys.platform == 'win32': - daemonize(start_options, timeout=args.timeout) + if daemonize(start_options, timeout=args.timeout): + sys.exit(1) else: if daemonize(Server(start_options, timeout=args.timeout).serve, @@ -198,7 +206,7 @@ def start_server(args: argparse.Namespace, allow_sources: bool = False) -> None: wait_for_server() -def wait_for_server(timeout: float = 5.0) -> None: +def wait_for_server(timeout: float = 30.0) -> None: """Wait until the server is up. Exit if it doesn't happen within the timeout. @@ -235,7 +243,6 @@ def do_run(args: argparse.Namespace) -> None: if not is_running(): # Bad or missing status file or dead process; good to start. start_server(args, allow_sources=True) - t0 = time.time() response = request('run', version=__version__, args=args.flags) # If the daemon signals that a restart is necessary, do it @@ -287,10 +294,9 @@ def do_kill(args: argparse.Namespace) -> None: pid, sockname = get_status() try: if sys.platform == 'win32': - sig = 1 # On Windows, kill is always sigkill, so this is the exit code + subprocess.check_output("taskkill /pid {pid} /f /t".format(pid=pid)) else: - sig = signal.SIGKILL - os.kill(pid, sig) + os.kill(pid, signal.SIGKILL) except OSError as err: sys.exit(str(err)) else: @@ -377,12 +383,12 @@ def do_daemon(args: argparse.Namespace) -> None: # Lazy import so this import doesn't slow down other commands. from mypy.dmypy_server import Server, process_start_options if args.options_file: - with open(args.options_file) as f: + with open(args.options_file, 'rb') as f: options, timeout = pickle.load(f) + os.unlink(args.options_file) else: options = process_start_options(args.flags, allow_sources=False) timeout = args.timeout - sys.exit(0) # FIXME: this is just for testing Server(options, timeout=timeout).serve() @@ -408,16 +414,17 @@ def request(command: str, *, timeout: Optional[float] = None, raised OSError. This covers cases such as connection refused or closed prematurely as well as invalid JSON received. """ + handle = _winapi.NULL + response = {} # type: Dict[str, str] args = dict(kwds) args.update(command=command) bdata = json.dumps(args).encode('utf8') pid, name = get_status() try: if sys.platform == 'win32': - # Sadly we need to poll until this works, or it times out - # thanks Windows - while True: + while 1: try: + _winapi.WaitNamedPipe(name, 5000) handle = _winapi.CreateFile( name, _winapi.GENERIC_READ | _winapi.GENERIC_WRITE, @@ -427,25 +434,30 @@ def request(command: str, *, timeout: Optional[float] = None, 0, _winapi.NULL, ) - _winapi.SetNamedPipeHandleState(handle, - _winapi.NULL, - _winapi.NULL, - _winapi.NULL) - except FileNotFoundError: - raise WindowsError( - "Unable to open connection to pipe at {}".format(name) - ) except WindowsError as e: if e.winerror not in (_winapi.ERROR_SEM_TIMEOUT, _winapi.ERROR_PIPE_BUSY): - return {'error'} - else: - time.sleep(1) - break + return {'error': "Error with pipe {}: {}".format(name, str(e))} + else: + break + try: + _winapi.SetNamedPipeHandleState(handle, + _winapi.PIPE_READMODE_MESSAGE, + None, + None) + except WindowsError as e: + return {'error': "Failed to set pipe state: {}".format(e.winerror)} # great, we have a named pipe connection! Now write our data - _winapi.WriteFile(handle, bdata, len(bdata)) + try: + _winapi.WriteFile(handle, bdata) + _winapi.WriteFile(handle, b'') + except WindowsError as e: + return {'error': "Failed to write with error: {}".format(e.winerror)} # now get the response - response = receive(handle) - + try: + response = receive(handle) + except WindowsError as e: + if e.winerror != 109: # The pipe ended (the server hung up) + raise else: sock = socket.socket(socket.AF_UNIX) if timeout is not None: @@ -461,7 +473,8 @@ def request(command: str, *, timeout: Optional[float] = None, return response finally: if sys.platform == 'win32': - _winapi.CloseHandle(handle) + if handle != _winapi.NULL: + _winapi.CloseHandle(handle) else: sock.close() @@ -489,10 +502,20 @@ def check_status(data: Dict[str, Any]) -> Tuple[int, str]: pid = data['pid'] if not isinstance(pid, int): raise BadStatus("pid field is not an int") - try: - os.kill(pid, 0) - except OSError: - raise BadStatus("Daemon has died") + if sys.platform == 'win32': + # why can't anything be easy... + status = wintypes.DWORD() + handle = OpenProcess(0x1000, # PROCESS_QUERY_LIMITED_INFORMATION + 0, + pid) + GetExitCodeProcess(handle, ctypes.byref(status)) + if status.value != 259: # STILL_ACTIVE + raise BadStatus("Daemon has died") + else: + try: + os.kill(pid, 0) + except OSError: + raise BadStatus("Daemon has died") if 'sockname' not in data: raise BadStatus("Invalid status file (no sockname field)") sockname = data['sockname'] diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index f7099b3c5ae3..761672441b2f 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -10,6 +10,7 @@ import json import os import pickle +import random import shutil import socket import subprocess @@ -56,13 +57,15 @@ def daemonize(opts: Options, timeout: Optional[int] = None) -> int: It also pickles the options to be unpickled by mypy. """ command = [sys.executable, '-m', 'mypy.dmypy', 'daemon'] - with tempfile.TemporaryDirectory() as dir: - options_file = os.path.join(dir, 'dmypy_options.pickle') - with open(options_file, 'wb') as file: - pickle.dump((opts, timeout), file) - command.append('--options-file={}'.format(options_file)) - print(command) - subprocess.run(command, creationflags=DETACHED_PROCESS) + options_file = ".dmypy_options.pickle" + with open(options_file, 'wb') as file: + pickle.dump((opts, timeout), file) + command.append('--options-file={}'.format(options_file)) + try: + subprocess.Popen(command, creationflags=DETACHED_PROCESS) + return 0 + except subprocess.CalledProcessError as e: + return e.returncode else: def daemonize(func: Callable[[], None], log_file: Optional[str] = None) -> int: @@ -204,41 +207,46 @@ def serve_win(self) -> None: if handle == -1: # INVALID_HANDLE_VALUE err = _winapi.GetLastError() print('Invalid handle to pipe: {err}'.format(err)) + os.unlink(STATUS_FILE) sys.exit(1) - res = _winapi.ConnectNamedPipe(handle, _winapi.NULL) - # we can connect if we connected, or the client already connected - if res or _winapi.GetLastError() == _winapi.ERROR_PIPE_CONNECTED: - while True: - try: - data = receive(handle) - except OSError: - continue - resp = {} # type: Dict[str, Any] - if 'command' not in data: - resp = {'error': "No command found in request"} + _winapi.ConnectNamedPipe(handle, _winapi.NULL) + while True: + try: + data = receive(handle) + except OSError: + continue + resp = {} # type: Dict[str, Any] + if 'command' not in data: + resp = {'error': "No command found in request"} + else: + command = data['command'] + if not isinstance(command, str): + resp = {'error': "Command is not a string"} else: - command = data['command'] - if not isinstance(command, str): - resp = {'error': "Command is not a string"} - else: - command = data.pop('command') - try: - resp = self.run_command(command, data) - except Exception: - # If we are crashing, report the crash to the client - tb = traceback.format_exception(*sys.exc_info()) - resp = {'error': "Daemon crashed!\n" + "".join(tb)} - _winapi.WriteFile(handle, json.dumps(resp).encode('utf8')) - raise - try: - _winapi.WriteFile(handle, json.dumps(resp).encode('utf8')) - except OSError: - pass # Maybe the client hung up - - if command == 'stop': + command = data.pop('command') + if command == 'stop': + _winapi.CloseHandle(handle) + os.unlink(STATUS_FILE) + reset_global_state() + sys.exit(0) + try: + resp = self.run_command(command, data) + except Exception: + # If we are crashing, report the crash to the client + tb = traceback.format_exception(*sys.exc_info()) + resp = {'error': "Daemon crashed!\n" + "".join(tb)} + _winapi.WriteFile(handle, json.dumps(resp).encode('utf8')) + _winapi.WriteFile(handle, b'') + raise + try: + _winapi.WriteFile(handle, json.dumps(resp).encode('utf8')) + _winapi.WriteFile(handle, b'') + if handle != _winapi.NULL: _winapi.CloseHandle(handle) - reset_global_state() - sys.exit(0) + break + except WindowsError as e: + print("Failed to respond to client: {}, {}".format(e, e.winerror)) + pass # Maybe the client hung up finally: os.unlink(STATUS_FILE) if handle != _winapi.NULL: diff --git a/mypy/dmypy_util.py b/mypy/dmypy_util.py index 9d6080a4c84e..2104558e5be4 100644 --- a/mypy/dmypy_util.py +++ b/mypy/dmypy_util.py @@ -32,7 +32,7 @@ def receive(connection: Union[socket.socket, HANDLE]) -> Any: read_size = 100000 if isinstance(connection, HANDLE): while True: - more = _winapi.ReadFile(connection, read_size) + more, _ = _winapi.ReadFile(connection, read_size) if not more: break bdata.extend(more) From 54859dac5eb3cd51cba798343b8ca40913205acb Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Tue, 30 Oct 2018 23:00:05 -0700 Subject: [PATCH 04/50] Only use _winapi on Windows --- mypy/dmypy.py | 3 ++- mypy/dmypy_util.py | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index 47777f58f02a..456ff1523aae 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -414,7 +414,8 @@ def request(command: str, *, timeout: Optional[float] = None, raised OSError. This covers cases such as connection refused or closed prematurely as well as invalid JSON received. """ - handle = _winapi.NULL + if sys.platform == 'win32': + handle = _winapi.NULL response = {} # type: Dict[str, str] args = dict(kwds) args.update(command=command) diff --git a/mypy/dmypy_util.py b/mypy/dmypy_util.py index 2104558e5be4..2765d3e59bc8 100644 --- a/mypy/dmypy_util.py +++ b/mypy/dmypy_util.py @@ -7,7 +7,8 @@ import socket import sys -import _winapi +if sys.platform == 'win32': + import _winapi from typing import Any, Union @@ -30,13 +31,14 @@ def receive(connection: Union[socket.socket, HANDLE]) -> Any: """ bdata = bytearray() read_size = 100000 - if isinstance(connection, HANDLE): + if sys.platform == 'win32' and isinstance(connection, HANDLE): while True: more, _ = _winapi.ReadFile(connection, read_size) if not more: break bdata.extend(more) else: + assert isinstance(connection, socket.socket) while True: more = connection.recv(read_size) if not more: From f760e1699beedd3f12a9269788c3a0225d9171b5 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Tue, 30 Oct 2018 23:15:29 -0700 Subject: [PATCH 05/50] Guard _winapi import dmypy_server --- mypy/dmypy_server.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 761672441b2f..970582adfaea 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -19,7 +19,8 @@ import time import traceback -import _winapi +if sys.platform == 'win32': + import _winapi from typing import AbstractSet, Any, Callable, Dict, List, Mapping, Optional, Sequence, Set, Tuple From 966117c9bd5ebca8d58db2924bb99f049d651476 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Tue, 30 Oct 2018 23:36:02 -0700 Subject: [PATCH 06/50] Get dmypy status working on Windows --- mypy/dmypy_server.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 970582adfaea..d27d7c05de37 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -542,17 +542,7 @@ def cmd_hang(self) -> Dict[str, object]: def get_meminfo() -> Dict[str, Any]: - # See https://stackoverflow.com/questions/938733/total-memory-used-by-python-process - import resource # Since it doesn't exist on Windows. res = {} # type: Dict[str, Any] - rusage = resource.getrusage(resource.RUSAGE_SELF) - # mypyc doesn't like unreachable code, so trick mypy into thinking the branch is reachable - if sys.platform == 'darwin' or bool(): - factor = 1 - else: - factor = 1024 # Linux - res['memory_maxrss_mib'] = rusage.ru_maxrss * factor / MiB - # If we can import psutil, use it for some extra data try: import psutil # type: ignore # It's not in typeshed yet except ImportError: @@ -566,4 +556,17 @@ def get_meminfo() -> Dict[str, Any]: meminfo = process.memory_info() res['memory_rss_mib'] = meminfo.rss / MiB res['memory_vms_mib'] = meminfo.vms / MiB + if sys.platform == 'win32': + res['memory_maxrss_mib'] = meminfo.peak_wset / MiB + else: + # See https://stackoverflow.com/questions/938733/total-memory-used-by-python-process + import resource # Since it doesn't exist on Windows. + rusage = resource.getrusage(resource.RUSAGE_SELF) + # mypyc doesn't like unreachable code, so trick mypy into thinking + # the branch is reachable + if sys.platform == 'darwin' or bool(): + factor = 1 + else: + factor = 1024 # Linux + res['memory_maxrss_mib'] = rusage.ru_maxrss * factor / MiB return res From 68ee937c276f317a4db930db2d753ab0d034e032 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Tue, 30 Oct 2018 23:38:39 -0700 Subject: [PATCH 07/50] Add support for timeout --- mypy/dmypy.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index 456ff1523aae..8e985311f8b4 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -401,7 +401,7 @@ def do_help(args: argparse.Namespace) -> None: # Client-side infrastructure. -def request(command: str, *, timeout: Optional[float] = None, +def request(command: str, *, timeout: Optional[int] = None, **kwds: object) -> Dict[str, Any]: """Send a request to the daemon. @@ -415,6 +415,7 @@ def request(command: str, *, timeout: Optional[float] = None, closed prematurely as well as invalid JSON received. """ if sys.platform == 'win32': + timeout = timeout or 1000 # we have to set a timeout handle = _winapi.NULL response = {} # type: Dict[str, str] args = dict(kwds) @@ -425,7 +426,7 @@ def request(command: str, *, timeout: Optional[float] = None, if sys.platform == 'win32': while 1: try: - _winapi.WaitNamedPipe(name, 5000) + _winapi.WaitNamedPipe(name, timeout * 1000) handle = _winapi.CreateFile( name, _winapi.GENERIC_READ | _winapi.GENERIC_WRITE, From f6a42c3e50e3966d2759cc548189602505959c99 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Tue, 30 Oct 2018 23:47:56 -0700 Subject: [PATCH 08/50] Fix lint on Linux --- mypy/dmypy_server.py | 122 ++++++++++++++++++++++--------------------- 1 file changed, 62 insertions(+), 60 deletions(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index d27d7c05de37..a9f24d154ae1 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -189,69 +189,71 @@ def serve(self) -> None: self.serve_nix() def serve_win(self) -> None: - name = r'\\.\pipe\python-{}-{}'.format(os.getpid(), SOCKET_NAME) - handle = _winapi.NULL - try: - with open(STATUS_FILE, 'w') as f: - json.dump({'pid': os.getpid(), 'sockname': name}, f) - f.write('\n') # I like my JSON with trailing newline - while True: - handle = _winapi.CreateNamedPipe(name, - _winapi.PIPE_ACCESS_DUPLEX | _winapi.FILE_FLAG_FIRST_PIPE_INSTANCE, - _winapi.PIPE_READMODE_MESSAGE | _winapi.PIPE_TYPE_MESSAGE | _winapi.PIPE_WAIT, - 1, # one instance - self.buffer_size, - self.buffer_size, - 0, # PIPE_WAIT, wait for a connection of the client - 0, # PIPE_ACCEPT_REMOTE_CLIENTS (this is the default, so I guess?) - ) - if handle == -1: # INVALID_HANDLE_VALUE - err = _winapi.GetLastError() - print('Invalid handle to pipe: {err}'.format(err)) - os.unlink(STATUS_FILE) - sys.exit(1) - _winapi.ConnectNamedPipe(handle, _winapi.NULL) + if sys.platform == 'win32': + name = r'\\.\pipe\python-{}-{}'.format(os.getpid(), SOCKET_NAME) + handle = _winapi.NULL + try: + with open(STATUS_FILE, 'w') as f: + json.dump({'pid': os.getpid(), 'sockname': name}, f) + f.write('\n') # I like my JSON with trailing newline while True: - try: - data = receive(handle) - except OSError: - continue - resp = {} # type: Dict[str, Any] - if 'command' not in data: - resp = {'error': "No command found in request"} - else: - command = data['command'] - if not isinstance(command, str): - resp = {'error': "Command is not a string"} + handle = _winapi.CreateNamedPipe(name, + _winapi.PIPE_ACCESS_DUPLEX | _winapi.FILE_FLAG_FIRST_PIPE_INSTANCE, + _winapi.PIPE_READMODE_MESSAGE | _winapi.PIPE_TYPE_MESSAGE + | _winapi.PIPE_WAIT, + 1, # one instance + self.buffer_size, + self.buffer_size, + 0, # PIPE_WAIT, wait for a connection of the client + 0, # PIPE_ACCEPT_REMOTE_CLIENTS (this is the default, so I guess?) + ) + if handle == -1: # INVALID_HANDLE_VALUE + err = _winapi.GetLastError() + print('Invalid handle to pipe: {err}'.format(err)) + os.unlink(STATUS_FILE) + sys.exit(1) + _winapi.ConnectNamedPipe(handle, _winapi.NULL) + while True: + try: + data = receive(handle) + except OSError: + continue + resp = {} # type: Dict[str, Any] + if 'command' not in data: + resp = {'error': "No command found in request"} else: - command = data.pop('command') - if command == 'stop': + command = data['command'] + if not isinstance(command, str): + resp = {'error': "Command is not a string"} + else: + command = data.pop('command') + if command == 'stop': + _winapi.CloseHandle(handle) + os.unlink(STATUS_FILE) + reset_global_state() + sys.exit(0) + try: + resp = self.run_command(command, data) + except Exception: + # If we are crashing, report the crash to the client + tb = traceback.format_exception(*sys.exc_info()) + resp = {'error': "Daemon crashed!\n" + "".join(tb)} + _winapi.WriteFile(handle, json.dumps(resp).encode('utf8')) + _winapi.WriteFile(handle, b'') + raise + try: + _winapi.WriteFile(handle, json.dumps(resp).encode('utf8')) + _winapi.WriteFile(handle, b'') + if handle != _winapi.NULL: _winapi.CloseHandle(handle) - os.unlink(STATUS_FILE) - reset_global_state() - sys.exit(0) - try: - resp = self.run_command(command, data) - except Exception: - # If we are crashing, report the crash to the client - tb = traceback.format_exception(*sys.exc_info()) - resp = {'error': "Daemon crashed!\n" + "".join(tb)} - _winapi.WriteFile(handle, json.dumps(resp).encode('utf8')) - _winapi.WriteFile(handle, b'') - raise - try: - _winapi.WriteFile(handle, json.dumps(resp).encode('utf8')) - _winapi.WriteFile(handle, b'') - if handle != _winapi.NULL: - _winapi.CloseHandle(handle) - break - except WindowsError as e: - print("Failed to respond to client: {}, {}".format(e, e.winerror)) - pass # Maybe the client hung up - finally: - os.unlink(STATUS_FILE) - if handle != _winapi.NULL: - _winapi.CloseHandle(handle) + break + except WindowsError as e: + print("Failed to respond to client: {}, {}".format(e, e.winerror)) + pass # Maybe the client hung up + finally: + os.unlink(STATUS_FILE) + if handle != _winapi.NULL: + _winapi.CloseHandle(handle) def serve_nix(self) -> None: try: From 168e17761bdd57a74c481c1fbd846fd569a002c9 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Wed, 31 Oct 2018 00:06:32 -0700 Subject: [PATCH 09/50] Change a while 1 to a while True --- mypy/dmypy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index 8e985311f8b4..b64588073f2c 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -424,7 +424,7 @@ def request(command: str, *, timeout: Optional[int] = None, pid, name = get_status() try: if sys.platform == 'win32': - while 1: + while True: try: _winapi.WaitNamedPipe(name, timeout * 1000) handle = _winapi.CreateFile( From 3e0d12ef82cc2a89bef8bd3f1acab8723b67285f Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Wed, 31 Oct 2018 00:16:04 -0700 Subject: [PATCH 10/50] Minor refactoring --- mypy/dmypy.py | 5 +++-- mypy/dmypy_server.py | 7 +++---- mypy/dmypy_util.py | 7 +++++++ 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index b64588073f2c..a4eac5a7ff7d 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -22,6 +22,8 @@ import ctypes from ctypes import wintypes + from mypy.dmypy_util import write_file + OpenProcess = ctypes.windll.kernel32.OpenProcess GetExitCodeProcess = ctypes.windll.kernel32.GetExitCodeProcess @@ -450,8 +452,7 @@ def request(command: str, *, timeout: Optional[int] = None, return {'error': "Failed to set pipe state: {}".format(e.winerror)} # great, we have a named pipe connection! Now write our data try: - _winapi.WriteFile(handle, bdata) - _winapi.WriteFile(handle, b'') + write_file(handle, bdata) except WindowsError as e: return {'error': "Failed to write with error: {}".format(e.winerror)} # now get the response diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index a9f24d154ae1..5c4081cb3ed9 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -21,6 +21,7 @@ if sys.platform == 'win32': import _winapi + from mypy.dmypy_util import write_file from typing import AbstractSet, Any, Callable, Dict, List, Mapping, Optional, Sequence, Set, Tuple @@ -238,12 +239,10 @@ def serve_win(self) -> None: # If we are crashing, report the crash to the client tb = traceback.format_exception(*sys.exc_info()) resp = {'error': "Daemon crashed!\n" + "".join(tb)} - _winapi.WriteFile(handle, json.dumps(resp).encode('utf8')) - _winapi.WriteFile(handle, b'') + write_file(handle, json.dumps(resp).encode('utf8')) raise try: - _winapi.WriteFile(handle, json.dumps(resp).encode('utf8')) - _winapi.WriteFile(handle, b'') + write_file(handle, json.dumps(resp).encode('utf8')) if handle != _winapi.NULL: _winapi.CloseHandle(handle) break diff --git a/mypy/dmypy_util.py b/mypy/dmypy_util.py index 2765d3e59bc8..67d19d45f3ef 100644 --- a/mypy/dmypy_util.py +++ b/mypy/dmypy_util.py @@ -20,6 +20,13 @@ HANDLE = int +if sys.platform == 'win32': + + def write_file(handle: HANDLE, data: bytes) -> None: + """Write some bytes to a HANDLE and then an empty string""" + _winapi.WriteFile(handle, data) + _winapi.WriteFile(handle, b'') + def receive(connection: Union[socket.socket, HANDLE]) -> Any: """Receive JSON data from a socket until EOF. From 87e61c5eae806dbe566f57484f686678cf0e40a1 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Wed, 31 Oct 2018 00:21:04 -0700 Subject: [PATCH 11/50] Move conditional imports to end of imports --- mypy/dmypy.py | 12 ++++++------ mypy/dmypy_server.py | 7 +++---- mypy/dmypy_util.py | 6 +++--- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index a4eac5a7ff7d..9e7b08cc57c3 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -16,6 +16,12 @@ import sys import time +from typing import Any, Callable, Dict, List, Mapping, Optional, Tuple + +from mypy.dmypy_util import STATUS_FILE, receive +from mypy.util import write_junit_xml +from mypy.version import __version__ + if sys.platform == 'win32': # This may be private, but it is needed for IPC on Windows, and is basically stable import _winapi @@ -27,12 +33,6 @@ OpenProcess = ctypes.windll.kernel32.OpenProcess GetExitCodeProcess = ctypes.windll.kernel32.GetExitCodeProcess -from typing import Any, Callable, Dict, List, Mapping, Optional, Tuple - -from mypy.dmypy_util import STATUS_FILE, receive -from mypy.util import write_junit_xml -from mypy.version import __version__ - # Argument parser. Subparsers are tied to action functions by the # @action(subparse) decorator. diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 5c4081cb3ed9..afdc3c906239 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -19,10 +19,6 @@ import time import traceback -if sys.platform == 'win32': - import _winapi - from mypy.dmypy_util import write_file - from typing import AbstractSet, Any, Callable, Dict, List, Mapping, Optional, Sequence, Set, Tuple import mypy.build @@ -42,6 +38,9 @@ if MYPY: from typing_extensions import Final +if sys.platform == 'win32': + import _winapi + from mypy.dmypy_util import write_file MEM_PROFILE = False # type: Final # If True, dump memory profile after initialization diff --git a/mypy/dmypy_util.py b/mypy/dmypy_util.py index 67d19d45f3ef..bdca4c9a27ae 100644 --- a/mypy/dmypy_util.py +++ b/mypy/dmypy_util.py @@ -7,15 +7,15 @@ import socket import sys -if sys.platform == 'win32': - import _winapi - from typing import Any, Union MYPY = False if MYPY: from typing_extensions import Final +if sys.platform == 'win32': + import _winapi + STATUS_FILE = '.dmypy.json' # type: Final HANDLE = int From 1883c31babd57bc03f22d66e171a6e7ef6decbac Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Wed, 31 Oct 2018 00:39:58 -0700 Subject: [PATCH 12/50] Minor fixes, corrections, simplifications --- mypy/dmypy_server.py | 13 ++++--------- mypy/dmypy_util.py | 26 +++++++++++--------------- 2 files changed, 15 insertions(+), 24 deletions(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index afdc3c906239..de64cd0b97cf 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -44,10 +44,6 @@ MEM_PROFILE = False # type: Final # If True, dump memory profile after initialization - -DETACHED_PROCESS = 0x00000008 - - if sys.platform == 'win32': def daemonize(opts: Options, timeout: Optional[int] = None) -> int: """Create the daemon process via "dmypy daemon" and pass options via file @@ -63,7 +59,7 @@ def daemonize(opts: Options, timeout: Optional[int] = None) -> int: pickle.dump((opts, timeout), file) command.append('--options-file={}'.format(options_file)) try: - subprocess.Popen(command, creationflags=DETACHED_PROCESS) + subprocess.Popen(command, creationflags=0x8) # DETACHED_PROCESS return 0 except subprocess.CalledProcessError as e: return e.returncode @@ -195,7 +191,7 @@ def serve_win(self) -> None: try: with open(STATUS_FILE, 'w') as f: json.dump({'pid': os.getpid(), 'sockname': name}, f) - f.write('\n') # I like my JSON with trailing newline + f.write('\n') # I like my JSON with a trailing newline while True: handle = _winapi.CreateNamedPipe(name, _winapi.PIPE_ACCESS_DUPLEX | _winapi.FILE_FLAG_FIRST_PIPE_INSTANCE, @@ -210,8 +206,7 @@ def serve_win(self) -> None: if handle == -1: # INVALID_HANDLE_VALUE err = _winapi.GetLastError() print('Invalid handle to pipe: {err}'.format(err)) - os.unlink(STATUS_FILE) - sys.exit(1) + break _winapi.ConnectNamedPipe(handle, _winapi.NULL) while True: try: @@ -261,7 +256,7 @@ def serve_nix(self) -> None: try: with open(STATUS_FILE, 'w') as f: json.dump({'pid': os.getpid(), 'sockname': sock.getsockname()}, f) - f.write('\n') # I like my JSON with trailing newline + f.write('\n') # I like my JSON with a trailing newline while True: try: conn, addr = sock.accept() diff --git a/mypy/dmypy_util.py b/mypy/dmypy_util.py index bdca4c9a27ae..6758de6da059 100644 --- a/mypy/dmypy_util.py +++ b/mypy/dmypy_util.py @@ -13,17 +13,15 @@ if MYPY: from typing_extensions import Final -if sys.platform == 'win32': - import _winapi - STATUS_FILE = '.dmypy.json' # type: Final HANDLE = int if sys.platform == 'win32': + import _winapi def write_file(handle: HANDLE, data: bytes) -> None: - """Write some bytes to a HANDLE and then an empty string""" + """Write some bytes to a HANDLE and then an empty bytes string""" _winapi.WriteFile(handle, data) _winapi.WriteFile(handle, b'') @@ -38,19 +36,17 @@ def receive(connection: Union[socket.socket, HANDLE]) -> Any: """ bdata = bytearray() read_size = 100000 - if sys.platform == 'win32' and isinstance(connection, HANDLE): - while True: + + while True: + if sys.platform == 'win32': + assert isinstance(connection, HANDLE) more, _ = _winapi.ReadFile(connection, read_size) - if not more: - break - bdata.extend(more) - else: - assert isinstance(connection, socket.socket) - while True: + else: + assert isinstance(connection, socket.socket) more = connection.recv(read_size) - if not more: - break - bdata.extend(more) + if not more: + break + bdata.extend(more) if not bdata: raise OSError("No data received") try: From 8fb72037aee46a9493ca28d5db0b759900e22de7 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Mon, 5 Nov 2018 09:38:50 -0800 Subject: [PATCH 13/50] Remove outdated docs --- docs/source/mypy_daemon.rst | 6 ------ 1 file changed, 6 deletions(-) diff --git a/docs/source/mypy_daemon.rst b/docs/source/mypy_daemon.rst index 9b97a027bcfe..602d659d6fcf 100644 --- a/docs/source/mypy_daemon.rst +++ b/docs/source/mypy_daemon.rst @@ -22,10 +22,6 @@ you'll find errors sooner. The mypy daemon is experimental. In particular, the command-line interface may change in future mypy releases. -.. note:: - - The mypy daemon currently supports macOS and Linux only. - .. note:: Each mypy daemon process supports one user and one set of source files, @@ -103,5 +99,3 @@ Limitations limitation. This can be defined through the command line or through a :ref:`configuration file `. - -* Windows is not supported. From 69d04dd27bffa4e3eefef8cae374e41677ea1e5f Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Wed, 7 Nov 2018 09:04:51 -0800 Subject: [PATCH 14/50] Respond to Guido's comments --- mypy/dmypy.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index 4ddea8b6e0e8..5a6b2926f80a 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -26,12 +26,15 @@ # This may be private, but it is needed for IPC on Windows, and is basically stable import _winapi import ctypes - from ctypes import wintypes + from ctypes.wintypes import DWORD, HANDLE from mypy.dmypy_util import write_file - OpenProcess = ctypes.windll.kernel32.OpenProcess - GetExitCodeProcess = ctypes.windll.kernel32.GetExitCodeProcess + PROCESS_QUERY_LIMITED_INFORMATION = ctypes.c_ulong(0x1000) + + kernel32 = ctypes.windll.kernel32 + OpenProcess = kernel32.OpenProcess # type: Callable[[DWORD, int, int], HANDLE] + GetExitCodeProcess = kernel32.GetExitCodeProcess # type: Callable[[HANDLE, Any], int] # Argument parser. Subparsers are tied to action functions by the # @action(subparse) decorator. @@ -106,7 +109,7 @@ def __init__(self, prog: str) -> None: help="Server shutdown timeout (in seconds)") p.add_argument('flags', metavar='FLAG', nargs='*', type=str, help="Regular mypy flags (precede with --)") -p.add_argument('--options-file', help="For internal use only") +p.add_argument('--options-file', help=argparse.SUPPRESS) help_parser = p = subparsers.add_parser('help') del p @@ -415,7 +418,7 @@ def request(command: str, *, timeout: Optional[int] = None, closed prematurely as well as invalid JSON received. """ if sys.platform == 'win32': - timeout = timeout or 1000 # we have to set a timeout + timeout = timeout or 1000000 # we have to set a timeout handle = _winapi.NULL response = {} # type: Dict[str, str] args = dict(kwds) @@ -505,8 +508,8 @@ def check_status(data: Dict[str, Any]) -> Tuple[int, str]: raise BadStatus("pid field is not an int") if sys.platform == 'win32': # why can't anything be easy... - status = wintypes.DWORD() - handle = OpenProcess(0x1000, # PROCESS_QUERY_LIMITED_INFORMATION + status = DWORD() + handle = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, 0, pid) GetExitCodeProcess(handle, ctypes.byref(status)) From 968e07cf4bbb30bbbe256c683ac38ced4efa72b1 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Thu, 8 Nov 2018 13:29:27 -0800 Subject: [PATCH 15/50] Lower timeout back down to 5s --- mypy/dmypy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index 5a6b2926f80a..ee05915a1e09 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -209,7 +209,7 @@ def start_server(args: argparse.Namespace, allow_sources: bool = False) -> None: wait_for_server() -def wait_for_server(timeout: float = 30.0) -> None: +def wait_for_server(timeout: float = 5.0) -> None: """Wait until the server is up. Exit if it doesn't happen within the timeout. From 037267a07677f988720ab6fac022e20403b822af Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Sun, 11 Nov 2018 21:59:36 -0800 Subject: [PATCH 16/50] Major refactoring into separate IPC module This consisted of: - extracting the socket and NamedPipe code into the mypy.ipc module - modifying mypy.dmypy and mypy.dmypy_server to use the new ipc server/client context manager for communication - testing the daemon on both platforms --- mypy/dmypy.py | 89 +++++---------------- mypy/dmypy_server.py | 130 +++++------------------------- mypy/dmypy_util.py | 30 ++----- mypy/ipc.py | 185 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 227 insertions(+), 207 deletions(-) create mode 100644 mypy/ipc.py diff --git a/mypy/dmypy.py b/mypy/dmypy.py index ee05915a1e09..28a5b148aeb3 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -19,17 +19,14 @@ from typing import Any, Callable, Dict, List, Mapping, Optional, Tuple from mypy.dmypy_util import STATUS_FILE, receive +from mypy.ipc import IPCClient, IPCException from mypy.util import write_junit_xml from mypy.version import __version__ if sys.platform == 'win32': - # This may be private, but it is needed for IPC on Windows, and is basically stable - import _winapi import ctypes from ctypes.wintypes import DWORD, HANDLE - from mypy.dmypy_util import write_file - PROCESS_QUERY_LIMITED_INFORMATION = ctypes.c_ulong(0x1000) kernel32 = ctypes.windll.kernel32 @@ -223,7 +220,7 @@ def wait_for_server(timeout: float = 5.0) -> None: time.sleep(0.1) continue # If the file's content is bogus or the process is dead, fail. - pid, sockname = check_status(data) + check_status(data) print("Daemon started") return sys.exit("Timed out waiting for daemon to start") @@ -294,7 +291,7 @@ def do_stop(args: argparse.Namespace) -> None: @action(kill_parser) def do_kill(args: argparse.Namespace) -> None: """Kill daemon process with SIGKILL.""" - pid, sockname = get_status() + pid, _ = get_status() try: if sys.platform == 'win32': subprocess.check_output("taskkill /pid {pid} /f /t".format(pid=pid)) @@ -417,76 +414,26 @@ def request(command: str, *, timeout: Optional[int] = None, raised OSError. This covers cases such as connection refused or closed prematurely as well as invalid JSON received. """ - if sys.platform == 'win32': - timeout = timeout or 1000000 # we have to set a timeout - handle = _winapi.NULL response = {} # type: Dict[str, str] args = dict(kwds) args.update(command=command) bdata = json.dumps(args).encode('utf8') - pid, name = get_status() + _, name = get_status() try: - if sys.platform == 'win32': - while True: - try: - _winapi.WaitNamedPipe(name, timeout * 1000) - handle = _winapi.CreateFile( - name, - _winapi.GENERIC_READ | _winapi.GENERIC_WRITE, - 0, - _winapi.NULL, - _winapi.OPEN_EXISTING, - 0, - _winapi.NULL, - ) - except WindowsError as e: - if e.winerror not in (_winapi.ERROR_SEM_TIMEOUT, _winapi.ERROR_PIPE_BUSY): - return {'error': "Error with pipe {}: {}".format(name, str(e))} - else: - break - try: - _winapi.SetNamedPipeHandleState(handle, - _winapi.PIPE_READMODE_MESSAGE, - None, - None) - except WindowsError as e: - return {'error': "Failed to set pipe state: {}".format(e.winerror)} - # great, we have a named pipe connection! Now write our data - try: - write_file(handle, bdata) - except WindowsError as e: - return {'error': "Failed to write with error: {}".format(e.winerror)} - # now get the response - try: - response = receive(handle) - except WindowsError as e: - if e.winerror != 109: # The pipe ended (the server hung up) - raise - else: - sock = socket.socket(socket.AF_UNIX) - if timeout is not None: - sock.settimeout(timeout) - sock.connect(name) - sock.sendall(bdata) - sock.shutdown(socket.SHUT_WR) - response = receive(sock) - except OSError as err: + with IPCClient(name, timeout) as client: + client.write(bdata) + response = receive(client) + except (OSError, IPCException) as err: return {'error': str(err)} # TODO: Other errors, e.g. ValueError, UnicodeError else: return response - finally: - if sys.platform == 'win32': - if handle != _winapi.NULL: - _winapi.CloseHandle(handle) - else: - sock.close() def get_status() -> Tuple[int, str]: """Read status file and check if the process is alive. - Return (pid, sockname) on success. + Return (pid, connection_name) on success. Raise BadStatus if something's wrong. """ @@ -497,7 +444,7 @@ def get_status() -> Tuple[int, str]: def check_status(data: Dict[str, Any]) -> Tuple[int, str]: """Check if the process is alive. - Return (pid, sockname) on success. + Return (pid, connection_name) on success. Raise BadStatus if something's wrong. """ @@ -514,18 +461,18 @@ def check_status(data: Dict[str, Any]) -> Tuple[int, str]: pid) GetExitCodeProcess(handle, ctypes.byref(status)) if status.value != 259: # STILL_ACTIVE - raise BadStatus("Daemon has died") + raise BadStatus("Daemon is dead") else: try: os.kill(pid, 0) except OSError: - raise BadStatus("Daemon has died") - if 'sockname' not in data: - raise BadStatus("Invalid status file (no sockname field)") - sockname = data['sockname'] - if not isinstance(sockname, str): - raise BadStatus("sockname field is not a string") - return pid, sockname + raise BadStatus("Daemon is dead") + if 'connection_name' not in data: + raise BadStatus("Invalid status file (no connection_name field)") + connection_name = data['connection_name'] + if not isinstance(connection_name, str): + raise BadStatus("connection_name field is not a string") + return pid, connection_name def read_status() -> Dict[str, object]: diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 4d5741b5d926..11b3f2902597 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -27,6 +27,7 @@ from mypy.find_sources import create_source_list, InvalidSourceList from mypy.server.update import FineGrainedBuildManager from mypy.dmypy_util import STATUS_FILE, receive +from mypy.ipc import IPCServer, IPCException from mypy.fscache import FileSystemCache from mypy.fswatcher import FileSystemWatcher, FileData from mypy.modulefinder import BuildSource, compute_search_paths @@ -38,10 +39,6 @@ if MYPY: from typing_extensions import Final -if sys.platform == 'win32': - import _winapi - from mypy.dmypy_util import write_file - MEM_PROFILE = False # type: Final # If True, dump memory profile after initialization if sys.platform == 'win32': @@ -112,8 +109,10 @@ def daemonize(func: Callable[[], None], log_file: Optional[str] = None) -> int: # Server code. - -SOCKET_NAME = 'dmypy.sock' # type: Final +if sys.platform == 'win32': + CONNECTION_NAME = r'\\.\pipe\dmypy.pipe' # type: Final +else: + CONNECTION_NAME = 'dmypy.sock' # type: Final def process_start_options(flags: List[str], allow_sources: bool) -> Options: @@ -179,96 +178,14 @@ def __init__(self, options: Options, def serve(self) -> None: """Serve requests, synchronously (no thread or fork).""" - if sys.platform == 'win32': - self.serve_win() - else: - self.serve_nix() - - def serve_win(self) -> None: - if sys.platform == 'win32': - name = r'\\.\pipe\python-{}-{}'.format(os.getpid(), SOCKET_NAME) - handle = _winapi.NULL - try: - with open(STATUS_FILE, 'w') as f: - json.dump({'pid': os.getpid(), 'sockname': name}, f) - f.write('\n') # I like my JSON with a trailing newline - while True: - handle = _winapi.CreateNamedPipe(name, - _winapi.PIPE_ACCESS_DUPLEX | _winapi.FILE_FLAG_FIRST_PIPE_INSTANCE, - _winapi.PIPE_READMODE_MESSAGE | _winapi.PIPE_TYPE_MESSAGE - | _winapi.PIPE_WAIT, - 1, # one instance - self.buffer_size, - self.buffer_size, - 0, # PIPE_WAIT, wait for a connection of the client - 0, # PIPE_ACCEPT_REMOTE_CLIENTS (this is the default, so I guess?) - ) - if handle == -1: # INVALID_HANDLE_VALUE - err = _winapi.GetLastError() - print('Invalid handle to pipe: {err}'.format(err)) - break - _winapi.ConnectNamedPipe(handle, _winapi.NULL) - while True: - try: - data = receive(handle) - except OSError: - continue - resp = {} # type: Dict[str, Any] - if 'command' not in data: - resp = {'error': "No command found in request"} - else: - command = data['command'] - if not isinstance(command, str): - resp = {'error': "Command is not a string"} - else: - command = data.pop('command') - if command == 'stop': - _winapi.CloseHandle(handle) - os.unlink(STATUS_FILE) - reset_global_state() - sys.exit(0) - try: - resp = self.run_command(command, data) - except Exception: - # If we are crashing, report the crash to the client - tb = traceback.format_exception(*sys.exc_info()) - resp = {'error': "Daemon crashed!\n" + "".join(tb)} - write_file(handle, json.dumps(resp).encode('utf8')) - raise - try: - write_file(handle, json.dumps(resp).encode('utf8')) - if handle != _winapi.NULL: - _winapi.CloseHandle(handle) - break - except WindowsError as e: - print("Failed to respond to client: {}, {}".format(e, e.winerror)) - pass # Maybe the client hung up - finally: - os.unlink(STATUS_FILE) - if handle != _winapi.NULL: - _winapi.CloseHandle(handle) - - def serve_nix(self) -> None: try: - sock = self.create_listening_socket() - if self.timeout is not None: - sock.settimeout(self.timeout) - try: - with open(STATUS_FILE, 'w') as f: - json.dump({'pid': os.getpid(), 'sockname': sock.getsockname()}, f) - f.write('\n') # I like my JSON with a trailing newline - while True: - try: - conn, addr = sock.accept() - except socket.timeout: - print("Exiting due to inactivity.") - reset_global_state() - sys.exit(0) - try: - data = receive(conn) - except OSError: - conn.close() # Maybe the client hung up - continue + server = IPCServer(CONNECTION_NAME, self.timeout) + with open(STATUS_FILE, 'w') as f: + json.dump({'pid': os.getpid(), 'connection_name': server.connection_name}, f) + f.write('\n') # I like my JSON with a trailing newline + while True: + with server: + data = receive(server) resp = {} # type: Dict[str, Any] if 'command' not in data: resp = {'error': "No command found in request"} @@ -284,34 +201,25 @@ def serve_nix(self) -> None: # If we are crashing, report the crash to the client tb = traceback.format_exception(*sys.exc_info()) resp = {'error': "Daemon crashed!\n" + "".join(tb)} - conn.sendall(json.dumps(resp).encode('utf8')) + server.write(json.dumps(resp).encode('utf8')) raise try: - conn.sendall(json.dumps(resp).encode('utf8')) + server.write(json.dumps(resp).encode('utf8')) except OSError: pass # Maybe the client hung up - conn.close() if command == 'stop': - sock.close() reset_global_state() sys.exit(0) - finally: - os.unlink(STATUS_FILE) finally: - shutil.rmtree(self.sock_directory) + try: + server.cleanup() # try to remove the socket dir on Linux + except OSError: + pass + os.unlink(STATUS_FILE) exc_info = sys.exc_info() if exc_info[0] and exc_info[0] is not SystemExit: traceback.print_exception(*exc_info) - def create_listening_socket(self) -> socket.socket: - """Create the socket and set it up for listening.""" - self.sock_directory = tempfile.mkdtemp() - sockname = os.path.join(self.sock_directory, SOCKET_NAME) - sock = socket.socket(socket.AF_UNIX) - sock.bind(sockname) - sock.listen(1) - return sock - def run_command(self, command: str, data: Mapping[str, object]) -> Dict[str, object]: """Run a specific command from the registry.""" key = 'cmd_' + command diff --git a/mypy/dmypy_util.py b/mypy/dmypy_util.py index 6758de6da059..4be8c07a5d5b 100644 --- a/mypy/dmypy_util.py +++ b/mypy/dmypy_util.py @@ -1,6 +1,6 @@ """Shared code between dmypy.py and dmypy_server.py. -This should be pretty lightweight and not depend on other mypy code. +This should be pretty lightweight and not depend on other mypy code (other than ipc). """ import json @@ -9,24 +9,16 @@ from typing import Any, Union +from mypy.ipc import IPCBase + MYPY = False if MYPY: from typing_extensions import Final STATUS_FILE = '.dmypy.json' # type: Final -HANDLE = int - -if sys.platform == 'win32': - import _winapi - - def write_file(handle: HANDLE, data: bytes) -> None: - """Write some bytes to a HANDLE and then an empty bytes string""" - _winapi.WriteFile(handle, data) - _winapi.WriteFile(handle, b'') - -def receive(connection: Union[socket.socket, HANDLE]) -> Any: +def receive(connection: IPCBase) -> Any: """Receive JSON data from a socket until EOF. Raise a subclass of OSError if there's a socket exception. @@ -34,19 +26,7 @@ def receive(connection: Union[socket.socket, HANDLE]) -> Any: Raise OSError if the data received is not valid JSON or if it is not a dict. """ - bdata = bytearray() - read_size = 100000 - - while True: - if sys.platform == 'win32': - assert isinstance(connection, HANDLE) - more, _ = _winapi.ReadFile(connection, read_size) - else: - assert isinstance(connection, socket.socket) - more = connection.recv(read_size) - if not more: - break - bdata.extend(more) + bdata = connection.read() if not bdata: raise OSError("No data received") try: diff --git a/mypy/ipc.py b/mypy/ipc.py new file mode 100644 index 000000000000..3d1ff824df73 --- /dev/null +++ b/mypy/ipc.py @@ -0,0 +1,185 @@ +"""Cross platform abstractions for inter-process communication + +On Unix, this uses AF_UNIX sockets. +On Windows, this uses NamedPipes. +""" + +import contextlib +import json +import os +import shutil +import socket +import sys +import tempfile + +from typing import Iterator, Optional, Type, Callable +from types import TracebackType + +if sys.platform == 'win32': + # This may be private, but it is needed for IPC on Windows, and is basically stable + import _winapi + HANDLE = int + + +class IPCException(Exception): + """Exception for IPC issues.""" + pass + + +class IPCBase: + """Base class for communication between the dmypy client and server. + + This contains logic shared between the client and server, such as reading + and writing. + """ + + if sys.platform == 'win32': + connection = None # type: int + else: + connection = None # type: socket.socket + + def __init__(self, name: str) -> None: + self.READ_SIZE = 100000 + self.name = name + + def read(self) -> bytes: + """Read bytes from an IPC connection until its empty.""" + bdata = bytearray() + while True: + if sys.platform == 'win32': + more, _ = _winapi.ReadFile(self.connection, self.READ_SIZE) + else: + more = self.connection.recv(self.READ_SIZE) + if not more: + break + bdata.extend(more) + return bytes(bdata) + + def write(self, data: bytes) -> None: + """Write bytes to an IPC connection.""" + if sys.platform == 'win32': + try: + _winapi.WriteFile(self.connection, data) + _winapi.WriteFile(self.connection, b'') + except WindowsError as e: + raise IPCException("Failed to write with error: {}".format(e.winerror)) + else: + self.connection.sendall(data) + self.connection.shutdown(socket.SHUT_WR) + + def close(self) -> None: + if sys.platform == 'win32': + if self.connection != _winapi.NULL: + _winapi.CloseHandle(self.connection) + else: + self.connection.close() + + +class IPCClient(IPCBase): + """The client side of an IPC connection.""" + + def __init__(self, name: str, timeout: Optional[int]) -> None: + super().__init__(name) + if sys.platform == 'win32': + timeout = timeout or 1000 # we need to set a timeout + while True: + try: + _winapi.WaitNamedPipe(self.name, timeout * 1000) + except WindowsError as e: + if e.winerror == _winapi.ERROR_SEM_TIMEOUT: + raise IPCException("Timed out waiting for connection.") + try: + self.connection = _winapi.CreateFile( + self.name, + _winapi.GENERIC_READ | _winapi.GENERIC_WRITE, + 0, + _winapi.NULL, + _winapi.OPEN_EXISTING, + 0, + _winapi.NULL, + ) + except WindowsError as e: + if e.winerror == _winapi.ERROR_PIPE_BUSY: + raise IPCException("The connection is busy.") + else: + break + _winapi.SetNamedPipeHandleState(self.connection, + _winapi.PIPE_READMODE_MESSAGE, + None, + None) + else: + self.connection = socket.socket(socket.AF_UNIX) + self.connection.settimeout(timeout) + self.connection.connect(name) + + def __enter__(self) -> 'IPCClient': + return self + + def __exit__(self, + exc_ty: Optional[Type[BaseException]] = None, + exc_val: Optional[BaseException] = None, + exc_tb: Optional[TracebackType] = None, + ) -> bool: + self.close() + return False + + +class IPCServer(IPCBase): + + BUFFER_SIZE = 2**16 + + def __init__(self, name: str, timeout: Optional[int] = None) -> None: + super().__init__(name) + if sys.platform != 'win32': + self.sock_directory = tempfile.mkdtemp() + sockfile = os.path.join(self.sock_directory, self.name) + self.sock = socket.socket(socket.AF_UNIX) + self.sock.bind(sockfile) + self.sock.listen(1) + if timeout is not None: + self.sock.settimeout(timeout) + + def __enter__(self) -> 'IPCServer': + if sys.platform == 'win32': + self.connection = _winapi.CreateNamedPipe(self.name, + _winapi.PIPE_ACCESS_DUPLEX | _winapi.FILE_FLAG_FIRST_PIPE_INSTANCE, + _winapi.PIPE_READMODE_MESSAGE | _winapi.PIPE_TYPE_MESSAGE | _winapi.PIPE_WAIT, + 1, # one instance + self.BUFFER_SIZE, + self.BUFFER_SIZE, + 0, # PIPE_WAIT, wait for a connection of the client + 0, # PIPE_ACCEPT_REMOTE_CLIENTS (this is the default, so I guess?) + ) + if self.connection == -1: # INVALID_HANDLE_VALUE + err = _winapi.GetLastError() + raise IPCException('Invalid handle to pipe: {err}'.format(err)) + # NOTE: It is theoretically possible that this will hang forever if the + # client never connects, though this can be "solved" by killing the server + _winapi.ConnectNamedPipe(self.connection, _winapi.NULL) + else: + while True: + try: + self.connection, _ = self.sock.accept() + break + except socket.timeout: + raise IPCException('The socket timed out') + return self + + def __exit__(self, + exc_ty: Optional[Type[BaseException]] = None, + exc_val: Optional[BaseException] = None, + exc_tb: Optional[TracebackType] = None, + ) -> bool: + self.close() + return False + + def cleanup(self) -> None: + if sys.platform != 'win32': + shutil.rmtree(self.sock_directory) + + @property + def connection_name(self) -> str: + if sys.platform == 'win32': + return self.name + else: + return self.sock.getsockname() From db202c231d5f09460473241fcb9215c833269200 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Sun, 11 Nov 2018 22:31:43 -0800 Subject: [PATCH 17/50] Fix typing import for Python 3.5.1 --- mypy/ipc.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/mypy/ipc.py b/mypy/ipc.py index 3d1ff824df73..7bbfc9e98fe9 100644 --- a/mypy/ipc.py +++ b/mypy/ipc.py @@ -12,7 +12,12 @@ import sys import tempfile -from typing import Iterator, Optional, Type, Callable +from typing import Iterator, Optional, Callable + +MYPY = False +if MYPY: + from typing import Type + from types import TracebackType if sys.platform == 'win32': From 55250a8db23187a49be637ddea1be05f142b5186 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Sun, 11 Nov 2018 22:40:59 -0800 Subject: [PATCH 18/50] Correct out of date comments and remove unused imports --- mypy/dmypy.py | 3 +-- mypy/dmypy_server.py | 3 --- mypy/dmypy_util.py | 4 +--- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index 28a5b148aeb3..89bec3cfb464 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -11,7 +11,6 @@ import os import pickle import signal -import socket import subprocess import sys import time @@ -410,7 +409,7 @@ def request(command: str, *, timeout: Optional[int] = None, Raise BadStatus if there is something wrong with the status file or if the process whose pid is in the status file has died. - Return {'error': } if a socket operation or receive() + Return {'error': } if an IPC operation or receive() raised OSError. This covers cases such as connection refused or closed prematurely as well as invalid JSON received. """ diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 11b3f2902597..134daaaac1bc 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -11,11 +11,8 @@ import os import pickle import random -import shutil -import socket import subprocess import sys -import tempfile import time import traceback diff --git a/mypy/dmypy_util.py b/mypy/dmypy_util.py index 4be8c07a5d5b..4d664a433680 100644 --- a/mypy/dmypy_util.py +++ b/mypy/dmypy_util.py @@ -19,9 +19,7 @@ def receive(connection: IPCBase) -> Any: - """Receive JSON data from a socket until EOF. - - Raise a subclass of OSError if there's a socket exception. + """Receive JSON data from a connection until EOF. Raise OSError if the data received is not valid JSON or if it is not a dict. From f066484d0a3996c2c7e06f36c60631e84eb574f8 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Sun, 11 Nov 2018 22:46:07 -0800 Subject: [PATCH 19/50] Quote types so they don't need to be imported at runtime --- mypy/ipc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mypy/ipc.py b/mypy/ipc.py index 7bbfc9e98fe9..3a963688e424 100644 --- a/mypy/ipc.py +++ b/mypy/ipc.py @@ -121,7 +121,7 @@ def __enter__(self) -> 'IPCClient': return self def __exit__(self, - exc_ty: Optional[Type[BaseException]] = None, + exc_ty: 'Optional[Type[BaseException]]' = None, exc_val: Optional[BaseException] = None, exc_tb: Optional[TracebackType] = None, ) -> bool: @@ -171,7 +171,7 @@ def __enter__(self) -> 'IPCServer': return self def __exit__(self, - exc_ty: Optional[Type[BaseException]] = None, + exc_ty: 'Optional[Type[BaseException]]' = None, exc_val: Optional[BaseException] = None, exc_tb: Optional[TracebackType] = None, ) -> bool: From 54cba38ff54531512c3b2db6d53d3cf31986b625 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Sun, 11 Nov 2018 22:47:22 -0800 Subject: [PATCH 20/50] Minor code cleanup --- mypy/ipc.py | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/mypy/ipc.py b/mypy/ipc.py index 3a963688e424..79cd96f5f5e2 100644 --- a/mypy/ipc.py +++ b/mypy/ipc.py @@ -79,6 +79,14 @@ def close(self) -> None: else: self.connection.close() + def __exit__(self, + exc_ty: 'Optional[Type[BaseException]]' = None, + exc_val: Optional[BaseException] = None, + exc_tb: Optional[TracebackType] = None, + ) -> bool: + self.close() + return False + class IPCClient(IPCBase): """The client side of an IPC connection.""" @@ -120,14 +128,6 @@ def __init__(self, name: str, timeout: Optional[int]) -> None: def __enter__(self) -> 'IPCClient': return self - def __exit__(self, - exc_ty: 'Optional[Type[BaseException]]' = None, - exc_val: Optional[BaseException] = None, - exc_tb: Optional[TracebackType] = None, - ) -> bool: - self.close() - return False - class IPCServer(IPCBase): @@ -146,6 +146,7 @@ def __init__(self, name: str, timeout: Optional[int] = None) -> None: def __enter__(self) -> 'IPCServer': if sys.platform == 'win32': + # The NamedPipe needs to be recreated every time we wait for a connection self.connection = _winapi.CreateNamedPipe(self.name, _winapi.PIPE_ACCESS_DUPLEX | _winapi.FILE_FLAG_FIRST_PIPE_INSTANCE, _winapi.PIPE_READMODE_MESSAGE | _winapi.PIPE_TYPE_MESSAGE | _winapi.PIPE_WAIT, @@ -170,14 +171,6 @@ def __enter__(self) -> 'IPCServer': raise IPCException('The socket timed out') return self - def __exit__(self, - exc_ty: 'Optional[Type[BaseException]]' = None, - exc_val: Optional[BaseException] = None, - exc_tb: Optional[TracebackType] = None, - ) -> bool: - self.close() - return False - def cleanup(self) -> None: if sys.platform != 'win32': shutil.rmtree(self.sock_directory) From 072518b1969124880bb02c5e151f3c5160694193 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Sun, 11 Nov 2018 22:49:42 -0800 Subject: [PATCH 21/50] Remove unused loop --- mypy/ipc.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/mypy/ipc.py b/mypy/ipc.py index 79cd96f5f5e2..7589352451e8 100644 --- a/mypy/ipc.py +++ b/mypy/ipc.py @@ -163,12 +163,10 @@ def __enter__(self) -> 'IPCServer': # client never connects, though this can be "solved" by killing the server _winapi.ConnectNamedPipe(self.connection, _winapi.NULL) else: - while True: - try: - self.connection, _ = self.sock.accept() - break - except socket.timeout: - raise IPCException('The socket timed out') + try: + self.connection, _ = self.sock.accept() + except socket.timeout: + raise IPCException('The socket timed out') return self def cleanup(self) -> None: From f033c5450b93b1768f85b4e559c4504b0592c6af Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Sun, 11 Nov 2018 23:10:26 -0800 Subject: [PATCH 22/50] Remove some more unused imports --- mypy/dmypy.py | 2 +- mypy/dmypy_util.py | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index 89bec3cfb464..0c5c99b5c20b 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -15,7 +15,7 @@ import sys import time -from typing import Any, Callable, Dict, List, Mapping, Optional, Tuple +from typing import Any, Callable, Dict, Mapping, Optional, Tuple from mypy.dmypy_util import STATUS_FILE, receive from mypy.ipc import IPCClient, IPCException diff --git a/mypy/dmypy_util.py b/mypy/dmypy_util.py index 4d664a433680..012b994cea18 100644 --- a/mypy/dmypy_util.py +++ b/mypy/dmypy_util.py @@ -4,10 +4,8 @@ """ import json -import socket -import sys -from typing import Any, Union +from typing import Any from mypy.ipc import IPCBase From 768a2bf51db772cce13326bfc8c22affc0905809 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Tue, 13 Nov 2018 23:54:30 -0800 Subject: [PATCH 23/50] Make pipe name and options file unique --- mypy/dmypy_server.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 134daaaac1bc..19ade5c885a3 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -13,6 +13,7 @@ import random import subprocess import sys +import tempfile import time import traceback @@ -48,10 +49,9 @@ def daemonize(opts: Options, timeout: Optional[int] = None) -> int: It also pickles the options to be unpickled by mypy. """ command = [sys.executable, '-m', 'mypy.dmypy', 'daemon'] - options_file = ".dmypy_options.pickle" - with open(options_file, 'wb') as file: + with tempfile.NamedTemporaryFile(suffix=".dmypy_options.pickle", delete=False) as file: pickle.dump((opts, timeout), file) - command.append('--options-file={}'.format(options_file)) + command.append('--options-file={}'.format(file.name)) try: subprocess.Popen(command, creationflags=0x8) # DETACHED_PROCESS return 0 @@ -107,7 +107,7 @@ def daemonize(func: Callable[[], None], log_file: Optional[str] = None) -> int: # Server code. if sys.platform == 'win32': - CONNECTION_NAME = r'\\.\pipe\dmypy.pipe' # type: Final + CONNECTION_NAME = r'\\.\pipe\dmypy-{}.pipe'.format(os.getpid()) # type: Final else: CONNECTION_NAME = 'dmypy.sock' # type: Final From 962072a9756680a2100c2c17594d023893905b71 Mon Sep 17 00:00:00 2001 From: "Michael J. Sullivan" Date: Thu, 15 Nov 2018 16:43:56 -0800 Subject: [PATCH 24/50] Move a conditional def out of IPCBase --- mypy/ipc.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/mypy/ipc.py b/mypy/ipc.py index 7589352451e8..8fabd6dd758a 100644 --- a/mypy/ipc.py +++ b/mypy/ipc.py @@ -5,10 +5,8 @@ """ import contextlib -import json import os import shutil -import socket import sys import tempfile @@ -23,14 +21,15 @@ if sys.platform == 'win32': # This may be private, but it is needed for IPC on Windows, and is basically stable import _winapi - HANDLE = int - + _IPCHandle = int +else: + import socket + _IPCHandle = socket.socket class IPCException(Exception): """Exception for IPC issues.""" pass - class IPCBase: """Base class for communication between the dmypy client and server. @@ -38,10 +37,7 @@ class IPCBase: and writing. """ - if sys.platform == 'win32': - connection = None # type: int - else: - connection = None # type: socket.socket + connection = None # type: _IPCHandle def __init__(self, name: str) -> None: self.READ_SIZE = 100000 From 8d909c5557f7c8c6350805540d4dcaee6f815201 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Thu, 15 Nov 2018 16:54:34 -0800 Subject: [PATCH 25/50] Remove dead code --- mypy/dmypy_server.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 19ade5c885a3..c0e54a5e9ae8 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -144,8 +144,6 @@ class Server: # NOTE: the instance is constructed in the parent process but # serve() is called in the grandchild (by daemonize()). - buffer_size = 2**16 - def __init__(self, options: Options, timeout: Optional[int] = None) -> None: """Initialize the server with the desired mypy flags.""" From eca5beb1674be293ff40c44f56e4844f7af24792 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Thu, 15 Nov 2018 17:15:31 -0800 Subject: [PATCH 26/50] Fix lint failure --- mypy/ipc.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mypy/ipc.py b/mypy/ipc.py index 8fabd6dd758a..831e8882e726 100644 --- a/mypy/ipc.py +++ b/mypy/ipc.py @@ -26,10 +26,12 @@ import socket _IPCHandle = socket.socket + class IPCException(Exception): """Exception for IPC issues.""" pass + class IPCBase: """Base class for communication between the dmypy client and server. From 941b1f27a01867a58a56d71a2322bf77711aa3bc Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Thu, 15 Nov 2018 18:33:21 -0800 Subject: [PATCH 27/50] Move to passing options via command instead of file --- mypy/dmypy.py | 12 +++++++----- mypy/dmypy_server.py | 7 ++++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index 0c5c99b5c20b..ddc85e31a3b4 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -7,6 +7,7 @@ """ import argparse +import base64 import json import os import pickle @@ -19,6 +20,7 @@ from mypy.dmypy_util import STATUS_FILE, receive from mypy.ipc import IPCClient, IPCException +from mypy.options import Options from mypy.util import write_junit_xml from mypy.version import __version__ @@ -105,7 +107,7 @@ def __init__(self, prog: str) -> None: help="Server shutdown timeout (in seconds)") p.add_argument('flags', metavar='FLAG', nargs='*', type=str, help="Regular mypy flags (precede with --)") -p.add_argument('--options-file', help=argparse.SUPPRESS) +p.add_argument('--options-data', help=argparse.SUPPRESS) help_parser = p = subparsers.add_parser('help') del p @@ -381,10 +383,10 @@ def do_daemon(args: argparse.Namespace) -> None: """Serve requests in the foreground.""" # Lazy import so this import doesn't slow down other commands. from mypy.dmypy_server import Server, process_start_options - if args.options_file: - with open(args.options_file, 'rb') as f: - options, timeout = pickle.load(f) - os.unlink(args.options_file) + if args.options_data: + options_dict, timeout = pickle.loads(base64.b64decode(args.options_data)) + options_obj = Options() + options = options_obj.apply_changes(options_dict) else: options = process_start_options(args.flags, allow_sources=False) timeout = args.timeout diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 163537799fce..6f708e4588d5 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -7,6 +7,7 @@ """ import argparse +import base64 import json import os import pickle @@ -33,6 +34,7 @@ from mypy.typestate import reset_global_state from mypy.version import __version__ + MYPY = False if MYPY: from typing_extensions import Final @@ -49,9 +51,8 @@ def daemonize(opts: Options, timeout: Optional[int] = None) -> int: It also pickles the options to be unpickled by mypy. """ command = [sys.executable, '-m', 'mypy.dmypy', 'daemon'] - with tempfile.NamedTemporaryFile(suffix=".dmypy_options.pickle", delete=False) as file: - pickle.dump((opts, timeout), file) - command.append('--options-file={}'.format(file.name)) + pickeled_options = pickle.dumps((opts.snapshot(), timeout)) + command.append('--options-data="{}"'.format(base64.b64encode(pickeled_options).decode())) try: subprocess.Popen(command, creationflags=0x8) # DETACHED_PROCESS return 0 From 06459a816854090dda59a7c18b43290574b24255 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Thu, 15 Nov 2018 18:56:57 -0800 Subject: [PATCH 28/50] Unify interface of daemonize --- mypy/dmypy.py | 19 +++++++++---------- mypy/dmypy_server.py | 14 +++++++++----- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index ddc85e31a3b4..6c3d88df9c07 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -194,16 +194,10 @@ def restart_server(args: argparse.Namespace, allow_sources: bool = False) -> Non def start_server(args: argparse.Namespace, allow_sources: bool = False) -> None: """Start the server from command arguments and wait for it.""" # Lazy import so this import doesn't slow down other commands. - from mypy.dmypy_server import daemonize, Server, process_start_options + from mypy.dmypy_server import daemonize, process_start_options start_options = process_start_options(args.flags, allow_sources) - if sys.platform == 'win32': - if daemonize(start_options, timeout=args.timeout): - sys.exit(1) - else: - if daemonize(Server(start_options, - timeout=args.timeout).serve, - args.log_file) != 0: - sys.exit(1) + if daemonize(start_options, timeout=args.timeout, log_file=args.log_file): + sys.exit(1) wait_for_server() @@ -384,9 +378,14 @@ def do_daemon(args: argparse.Namespace) -> None: # Lazy import so this import doesn't slow down other commands. from mypy.dmypy_server import Server, process_start_options if args.options_data: - options_dict, timeout = pickle.loads(base64.b64decode(args.options_data)) + options_dict, timeout, log_file = pickle.loads(base64.b64decode(args.options_data)) options_obj = Options() options = options_obj.apply_changes(options_dict) + if log_file: + sys.stdout = sys.stderr = open(log_file, 'a', buffering=1) + fd = sys.stdout.fileno() + os.dup2(fd, 2) + os.dup2(fd, 1) else: options = process_start_options(args.flags, allow_sources=False) timeout = args.timeout diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 6f708e4588d5..423e9b9fac8c 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -42,8 +42,10 @@ MEM_PROFILE = False # type: Final # If True, dump memory profile after initialization if sys.platform == 'win32': - def daemonize(opts: Options, timeout: Optional[int] = None) -> int: - """Create the daemon process via "dmypy daemon" and pass options via file + def daemonize(options: Options, + timeout: Optional[int] = None, + log_file: Optional[str] = None) -> int: + """Create the daemon process via "dmypy daemon" and pass options via command line This uses the DETACHED_PROCESS flag to invoke the Server. See https://docs.microsoft.com/en-us/windows/desktop/procthread/process-creation-flags @@ -51,7 +53,7 @@ def daemonize(opts: Options, timeout: Optional[int] = None) -> int: It also pickles the options to be unpickled by mypy. """ command = [sys.executable, '-m', 'mypy.dmypy', 'daemon'] - pickeled_options = pickle.dumps((opts.snapshot(), timeout)) + pickeled_options = pickle.dumps((options.snapshot(), timeout, log_file)) command.append('--options-data="{}"'.format(base64.b64encode(pickeled_options).decode())) try: subprocess.Popen(command, creationflags=0x8) # DETACHED_PROCESS @@ -60,7 +62,9 @@ def daemonize(opts: Options, timeout: Optional[int] = None) -> int: return e.returncode else: - def daemonize(func: Callable[[], None], log_file: Optional[str] = None) -> int: + def daemonize(options: Options, + timeout: Optional[int] = None, + log_file: Optional[str] = None) -> int: """Arrange to call func() in a grandchild of the current process. Return 0 for success, exit status for failure, negative if @@ -100,7 +104,7 @@ def daemonize(func: Callable[[], None], log_file: Optional[str] = None) -> int: fd = sys.stdout.fileno() os.dup2(fd, 2) os.dup2(fd, 1) - func() + Server(options, timeout).serve finally: # Make sure we never get back into the caller. os._exit(1) From 1feaf0e4beb38dec1b4de40407d74831cbc77c84 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Thu, 15 Nov 2018 19:01:02 -0800 Subject: [PATCH 29/50] Add comment about IPC behavior --- mypy/ipc.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mypy/ipc.py b/mypy/ipc.py index 831e8882e726..86bcbc0e1275 100644 --- a/mypy/ipc.py +++ b/mypy/ipc.py @@ -63,6 +63,8 @@ def write(self, data: bytes) -> None: if sys.platform == 'win32': try: _winapi.WriteFile(self.connection, data) + # this empty write is to copy the behavior of socket.sendall, + # which also sends an empty message to signify it is done writing _winapi.WriteFile(self.connection, b'') except WindowsError as e: raise IPCException("Failed to write with error: {}".format(e.winerror)) From 34bd86a4f5c15498befb24cf1f0a9a90ae34894e Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Thu, 15 Nov 2018 19:17:22 -0800 Subject: [PATCH 30/50] Move process status checking and killing into dmypy_os --- mypy/dmypy.py | 32 ++++---------------------------- mypy/dmypy_os.py | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 28 deletions(-) create mode 100644 mypy/dmypy_os.py diff --git a/mypy/dmypy.py b/mypy/dmypy.py index 6c3d88df9c07..f7a9c4e5d001 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -19,21 +19,12 @@ from typing import Any, Callable, Dict, Mapping, Optional, Tuple from mypy.dmypy_util import STATUS_FILE, receive +from mypy.dmypy_os import alive, kill from mypy.ipc import IPCClient, IPCException from mypy.options import Options from mypy.util import write_junit_xml from mypy.version import __version__ -if sys.platform == 'win32': - import ctypes - from ctypes.wintypes import DWORD, HANDLE - - PROCESS_QUERY_LIMITED_INFORMATION = ctypes.c_ulong(0x1000) - - kernel32 = ctypes.windll.kernel32 - OpenProcess = kernel32.OpenProcess # type: Callable[[DWORD, int, int], HANDLE] - GetExitCodeProcess = kernel32.GetExitCodeProcess # type: Callable[[HANDLE, Any], int] - # Argument parser. Subparsers are tied to action functions by the # @action(subparse) decorator. @@ -288,10 +279,7 @@ def do_kill(args: argparse.Namespace) -> None: """Kill daemon process with SIGKILL.""" pid, _ = get_status() try: - if sys.platform == 'win32': - subprocess.check_output("taskkill /pid {pid} /f /t".format(pid=pid)) - else: - os.kill(pid, signal.SIGKILL) + kill(pid) except OSError as err: sys.exit(str(err)) else: @@ -453,20 +441,8 @@ def check_status(data: Dict[str, Any]) -> Tuple[int, str]: pid = data['pid'] if not isinstance(pid, int): raise BadStatus("pid field is not an int") - if sys.platform == 'win32': - # why can't anything be easy... - status = DWORD() - handle = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, - 0, - pid) - GetExitCodeProcess(handle, ctypes.byref(status)) - if status.value != 259: # STILL_ACTIVE - raise BadStatus("Daemon is dead") - else: - try: - os.kill(pid, 0) - except OSError: - raise BadStatus("Daemon is dead") + if not alive(pid): + raise BadStatus("Daemon is dead") if 'connection_name' not in data: raise BadStatus("Invalid status file (no connection_name field)") connection_name = data['connection_name'] diff --git a/mypy/dmypy_os.py b/mypy/dmypy_os.py new file mode 100644 index 000000000000..77cf963ad612 --- /dev/null +++ b/mypy/dmypy_os.py @@ -0,0 +1,43 @@ +import sys + +from typing import Any, Callable + +if sys.platform == 'win32': + import ctypes + from ctypes.wintypes import DWORD, HANDLE + import subprocess + + PROCESS_QUERY_LIMITED_INFORMATION = ctypes.c_ulong(0x1000) + + kernel32 = ctypes.windll.kernel32 + OpenProcess = kernel32.OpenProcess # type: Callable[[DWORD, int, int], HANDLE] + GetExitCodeProcess = kernel32.GetExitCodeProcess # type: Callable[[HANDLE, Any], int] +else: + import os + import signal + + +def alive(pid: int) -> bool: + """Is the process alive?""" + if sys.platform == 'win32': + # why can't anything be easy... + status = DWORD() + handle = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, + 0, + pid) + GetExitCodeProcess(handle, ctypes.byref(status)) + return status.value == 259 # STILL_ACTIVE + else: + try: + os.kill(pid, 0) + except OSError: + return False + return True + + +def kill(pid: int) -> None: + """Kill the process.""" + if sys.platform == 'win32': + subprocess.check_output("taskkill /pid {pid} /f /t".format(pid=pid)) + else: + os.kill(pid, signal.SIGKILL) From c4d0badcb5b1f142d323984a58aa5e7260f796a1 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Fri, 16 Nov 2018 00:08:36 -0800 Subject: [PATCH 31/50] Add test for IPC --- mypy/ipc.py | 2 +- mypy/test/testipc.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 mypy/test/testipc.py diff --git a/mypy/ipc.py b/mypy/ipc.py index 86bcbc0e1275..7dde7b238255 100644 --- a/mypy/ipc.py +++ b/mypy/ipc.py @@ -97,7 +97,7 @@ def __init__(self, name: str, timeout: Optional[int]) -> None: timeout = timeout or 1000 # we need to set a timeout while True: try: - _winapi.WaitNamedPipe(self.name, timeout * 1000) + _winapi.WaitNamedPipe(self.name, timeout) except WindowsError as e: if e.winerror == _winapi.ERROR_SEM_TIMEOUT: raise IPCException("Timed out waiting for connection.") diff --git a/mypy/test/testipc.py b/mypy/test/testipc.py new file mode 100644 index 000000000000..e05f1621c775 --- /dev/null +++ b/mypy/test/testipc.py @@ -0,0 +1,34 @@ +import os +import sys +from unittest import TestCase, main +from multiprocessing import Process + +from mypy.ipc import IPCClient, IPCServer, IPCException + + +if sys.platform == 'win32': + CONNECTION_NAME = r'\\.\pipe\dmypy-ipc-test.pipe' # type: Final +else: + CONNECTION_NAME = 'dmypy-test-ipc.sock' # type: Final + + +def server(msg: str) -> None: + with IPCServer(CONNECTION_NAME) as server: + server.read() + server.write(msg.encode()) + server.cleanup() + + +class IPCTests(TestCase): + def test_transaction_large(self): + msg = 't' * 100001 # longer than the max read size of 100_000 + p = Process(target=server, args=(msg,)) + p.start() + with IPCClient(CONNECTION_NAME, timeout=10) as client: + client.write(b'') # signal we are ready for a write from the server + assert client.read() == msg.encode() + p.join() + + +if __name__ == '__main__': + main() From ed4548d91d0590eadfe1fb23b7572973f1884573 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Fri, 16 Nov 2018 00:20:31 -0800 Subject: [PATCH 32/50] Handle ERROR_PIPE_CONNECTED --- mypy/ipc.py | 6 +++++- mypy/test/testipc.py | 15 ++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/mypy/ipc.py b/mypy/ipc.py index 7dde7b238255..abf018edd7e4 100644 --- a/mypy/ipc.py +++ b/mypy/ipc.py @@ -161,7 +161,11 @@ def __enter__(self) -> 'IPCServer': raise IPCException('Invalid handle to pipe: {err}'.format(err)) # NOTE: It is theoretically possible that this will hang forever if the # client never connects, though this can be "solved" by killing the server - _winapi.ConnectNamedPipe(self.connection, _winapi.NULL) + try: + _winapi.ConnectNamedPipe(self.connection, _winapi.NULL) + except WindowsError as e: + if e.winerror == _winapi.ERROR_PIPE_CONNECTED: + pass # The client already exists, which is fine. else: try: self.connection, _ = self.sock.accept() diff --git a/mypy/test/testipc.py b/mypy/test/testipc.py index e05f1621c775..9f5ebffc3368 100644 --- a/mypy/test/testipc.py +++ b/mypy/test/testipc.py @@ -7,24 +7,25 @@ if sys.platform == 'win32': - CONNECTION_NAME = r'\\.\pipe\dmypy-ipc-test.pipe' # type: Final + CONNECTION_NAME = r'\\.\pipe\dmypy-ipc-test-{}.pipe' else: - CONNECTION_NAME = 'dmypy-test-ipc.sock' # type: Final + CONNECTION_NAME = 'dmypy-test-ipc-{}.sock' -def server(msg: str) -> None: - with IPCServer(CONNECTION_NAME) as server: +def server(msg: str, pid: int) -> None: + with IPCServer(CONNECTION_NAME.format(pid)) as server: server.read() server.write(msg.encode()) server.cleanup() class IPCTests(TestCase): - def test_transaction_large(self): + def test_transaction_large(self) -> None: + pid = os.getpid() msg = 't' * 100001 # longer than the max read size of 100_000 - p = Process(target=server, args=(msg,)) + p = Process(target=server, args=(msg, pid), daemon=True) p.start() - with IPCClient(CONNECTION_NAME, timeout=10) as client: + with IPCClient(CONNECTION_NAME.format(pid), timeout=10) as client: client.write(b'') # signal we are ready for a write from the server assert client.read() == msg.encode() p.join() From 681408fbde43bb130aa4c1398a2e751fdd68733e Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Fri, 16 Nov 2018 00:42:24 -0800 Subject: [PATCH 33/50] Give the server more time to get set up in testipc? --- mypy/test/testipc.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mypy/test/testipc.py b/mypy/test/testipc.py index 9f5ebffc3368..a9c832f1d4b6 100644 --- a/mypy/test/testipc.py +++ b/mypy/test/testipc.py @@ -1,5 +1,6 @@ import os import sys +import time from unittest import TestCase, main from multiprocessing import Process @@ -25,10 +26,10 @@ def test_transaction_large(self) -> None: msg = 't' * 100001 # longer than the max read size of 100_000 p = Process(target=server, args=(msg, pid), daemon=True) p.start() + time.sleep(1) # wait for server to get set up with IPCClient(CONNECTION_NAME.format(pid), timeout=10) as client: client.write(b'') # signal we are ready for a write from the server assert client.read() == msg.encode() - p.join() if __name__ == '__main__': From f484143c614a57d41335be6ad5c8a30841c10900 Mon Sep 17 00:00:00 2001 From: "Michael J. Sullivan" Date: Fri, 16 Nov 2018 11:46:12 -0800 Subject: [PATCH 34/50] Fix unix daemonize typo (and slightly regeneralize) --- mypy/dmypy_server.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 423e9b9fac8c..04e3c365b215 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -62,9 +62,7 @@ def daemonize(options: Options, return e.returncode else: - def daemonize(options: Options, - timeout: Optional[int] = None, - log_file: Optional[str] = None) -> int: + def _daemonize_cb(func: Callable[[], None], log_file: Optional[str] = None) -> int: """Arrange to call func() in a grandchild of the current process. Return 0 for success, exit status for failure, negative if @@ -104,11 +102,20 @@ def daemonize(options: Options, fd = sys.stdout.fileno() os.dup2(fd, 2) os.dup2(fd, 1) - Server(options, timeout).serve + func() finally: # Make sure we never get back into the caller. os._exit(1) + def daemonize(options: Options, + timeout: Optional[int] = None, + log_file: Optional[str] = None) -> int: + """Run the mypy daemon in a grandchild of the current process + + Return 0 for success, exit status for failure, negative if + subprocess killed by signal. + """ + return _daemonize_cb(Server(options, timeout).serve, log_file) # Server code. if sys.platform == 'win32': From 512c60e9637338154306e8dbf133be66785089c6 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Fri, 16 Nov 2018 14:25:59 -0800 Subject: [PATCH 35/50] Fix IPC tests --- mypy/test/testipc.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/mypy/test/testipc.py b/mypy/test/testipc.py index a9c832f1d4b6..af4bae79f044 100644 --- a/mypy/test/testipc.py +++ b/mypy/test/testipc.py @@ -2,7 +2,7 @@ import sys import time from unittest import TestCase, main -from multiprocessing import Process +from multiprocessing import Process, Queue from mypy.ipc import IPCClient, IPCServer, IPCException @@ -13,23 +13,26 @@ CONNECTION_NAME = 'dmypy-test-ipc-{}.sock' -def server(msg: str, pid: int) -> None: - with IPCServer(CONNECTION_NAME.format(pid)) as server: - server.read() +def server(msg: str, q: Queue) -> None: + server = IPCServer(CONNECTION_NAME.format(os.getpid())) + q.put(server.connection_name) + with server: server.write(msg.encode()) - server.cleanup() + server.cleanup() class IPCTests(TestCase): def test_transaction_large(self) -> None: - pid = os.getpid() + queue = Queue() msg = 't' * 100001 # longer than the max read size of 100_000 - p = Process(target=server, args=(msg, pid), daemon=True) + p = Process(target=server, args=(msg, queue), daemon=True) p.start() - time.sleep(1) # wait for server to get set up - with IPCClient(CONNECTION_NAME.format(pid), timeout=10) as client: - client.write(b'') # signal we are ready for a write from the server + connection_name = queue.get() + with IPCClient(connection_name, timeout=10) as client: assert client.read() == msg.encode() + queue.close() + queue.join_thread() + p.join() if __name__ == '__main__': From 09b491347bff6c6c2c96f255b8ab9e14f1f6d8d1 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Fri, 16 Nov 2018 14:30:06 -0800 Subject: [PATCH 36/50] Fix typecheck of ipc tests --- mypy/test/testipc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mypy/test/testipc.py b/mypy/test/testipc.py index af4bae79f044..5d94b045b431 100644 --- a/mypy/test/testipc.py +++ b/mypy/test/testipc.py @@ -13,7 +13,7 @@ CONNECTION_NAME = 'dmypy-test-ipc-{}.sock' -def server(msg: str, q: Queue) -> None: +def server(msg: str, q: 'Queue[str]') -> None: server = IPCServer(CONNECTION_NAME.format(os.getpid())) q.put(server.connection_name) with server: @@ -23,7 +23,7 @@ def server(msg: str, q: Queue) -> None: class IPCTests(TestCase): def test_transaction_large(self) -> None: - queue = Queue() + queue = Queue() # type: Queue[str] msg = 't' * 100001 # longer than the max read size of 100_000 p = Process(target=server, args=(msg, queue), daemon=True) p.start() From d2695ca4c8036cd83307333565b8018d6759c6be Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Fri, 16 Nov 2018 16:32:55 -0800 Subject: [PATCH 37/50] Reraise if exceptions occure and unify IPC initialization --- mypy/dmypy_server.py | 6 ++---- mypy/ipc.py | 6 ++++++ mypy/test/testipc.py | 9 +++------ 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 04e3c365b215..24f6d1eacba3 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -118,10 +118,8 @@ def daemonize(options: Options, return _daemonize_cb(Server(options, timeout).serve, log_file) # Server code. -if sys.platform == 'win32': - CONNECTION_NAME = r'\\.\pipe\dmypy-{}.pipe'.format(os.getpid()) # type: Final -else: - CONNECTION_NAME = 'dmypy.sock' # type: Final + +CONNECTION_NAME = 'dmypy.sock' # type: Final def process_start_options(flags: List[str], allow_sources: bool) -> Options: diff --git a/mypy/ipc.py b/mypy/ipc.py index abf018edd7e4..5d250de6182d 100644 --- a/mypy/ipc.py +++ b/mypy/ipc.py @@ -101,6 +101,8 @@ def __init__(self, name: str, timeout: Optional[int]) -> None: except WindowsError as e: if e.winerror == _winapi.ERROR_SEM_TIMEOUT: raise IPCException("Timed out waiting for connection.") + else: + raise try: self.connection = _winapi.CreateFile( self.name, @@ -114,6 +116,8 @@ def __init__(self, name: str, timeout: Optional[int]) -> None: except WindowsError as e: if e.winerror == _winapi.ERROR_PIPE_BUSY: raise IPCException("The connection is busy.") + else: + raise else: break _winapi.SetNamedPipeHandleState(self.connection, @@ -134,6 +138,8 @@ class IPCServer(IPCBase): BUFFER_SIZE = 2**16 def __init__(self, name: str, timeout: Optional[int] = None) -> None: + if sys.platform == 'win32': + name = r'\\.\pipe\{}-{}.pipe'.format(name, os.getpid()) super().__init__(name) if sys.platform != 'win32': self.sock_directory = tempfile.mkdtemp() diff --git a/mypy/test/testipc.py b/mypy/test/testipc.py index 5d94b045b431..5ffbe49b1e40 100644 --- a/mypy/test/testipc.py +++ b/mypy/test/testipc.py @@ -7,14 +7,11 @@ from mypy.ipc import IPCClient, IPCServer, IPCException -if sys.platform == 'win32': - CONNECTION_NAME = r'\\.\pipe\dmypy-ipc-test-{}.pipe' -else: - CONNECTION_NAME = 'dmypy-test-ipc-{}.sock' +CONNECTION_NAME = 'dmypy-test-ipc.sock' def server(msg: str, q: 'Queue[str]') -> None: - server = IPCServer(CONNECTION_NAME.format(os.getpid())) + server = IPCServer(CONNECTION_NAME) q.put(server.connection_name) with server: server.write(msg.encode()) @@ -28,7 +25,7 @@ def test_transaction_large(self) -> None: p = Process(target=server, args=(msg, queue), daemon=True) p.start() connection_name = queue.get() - with IPCClient(connection_name, timeout=10) as client: + with IPCClient(connection_name, timeout=1) as client: assert client.read() == msg.encode() queue.close() queue.join_thread() From 4956188456f1690afe9ee1e1bceb8b1a118d3c38 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Mon, 19 Nov 2018 23:08:33 -0800 Subject: [PATCH 38/50] Get tests passing on Windows --- mypy/dmypy.py | 2 +- mypy/test/testdaemon.py | 2 -- test-data/unit/daemon.test | 6 +++--- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index cc354994aa5b..f5f7f1d569aa 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -443,7 +443,7 @@ def check_status(data: Dict[str, Any]) -> Tuple[int, str]: if not isinstance(pid, int): raise BadStatus("pid field is not an int") if not alive(pid): - raise BadStatus("Daemon is dead") + raise BadStatus("Daemon has died") if 'connection_name' not in data: raise BadStatus("Invalid status file (no connection_name field)") connection_name = data['connection_name'] diff --git a/mypy/test/testdaemon.py b/mypy/test/testdaemon.py index 29135662c991..73f60372cd74 100644 --- a/mypy/test/testdaemon.py +++ b/mypy/test/testdaemon.py @@ -31,8 +31,6 @@ def run_case(self, testcase: DataDrivenTestCase) -> None: def test_daemon(testcase: DataDrivenTestCase) -> None: - if sys.platform == 'win32': - return # These tests don't run on Windows yet. assert testcase.old_cwd is not None, "test was not properly set up" for i, step in enumerate(parse_script(testcase.input)): cmd = step[0] diff --git a/test-data/unit/daemon.test b/test-data/unit/daemon.test index 7b2a42862725..9af5ddb91507 100644 --- a/test-data/unit/daemon.test +++ b/test-data/unit/daemon.test @@ -29,8 +29,8 @@ def f(): pass $ dmypy run -- foo.py --follow-imports=error Daemon started $ dmypy run -- foo.py --follow-imports=error -$ echo '[mypy]' >mypy.ini -$ echo 'disallow_untyped_defs = True' >>mypy.ini +$ echo [mypy] >mypy.ini +$ echo disallow_untyped_defs = True >>mypy.ini $ dmypy run -- foo.py --follow-imports=error Restarting: configuration changed Daemon stopped @@ -47,7 +47,7 @@ def f(): pass [case testDaemonRunRestartPluginVersion] $ dmypy run -- foo.py Daemon started -$ echo ' ' >>plug.py +$ echo # >>plug.py $ dmypy run -- foo.py Restarting: plugins changed Daemon stopped From 842712e4eac6b36ed79a4b183701ab0bec991b53 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Mon, 19 Nov 2018 23:29:15 -0800 Subject: [PATCH 39/50] Try an empty string/quotes instead of single quotes --- test-data/unit/daemon.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-data/unit/daemon.test b/test-data/unit/daemon.test index 9af5ddb91507..60f209c20b8b 100644 --- a/test-data/unit/daemon.test +++ b/test-data/unit/daemon.test @@ -47,7 +47,7 @@ def f(): pass [case testDaemonRunRestartPluginVersion] $ dmypy run -- foo.py Daemon started -$ echo # >>plug.py +$ echo "" >> plug.py $ dmypy run -- foo.py Restarting: plugins changed Daemon stopped From 5d9e1498ea3ca9a8e8cc3a43d058521e0228ae5f Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Tue, 20 Nov 2018 14:05:15 -0800 Subject: [PATCH 40/50] Really fix daemon tests --- mypy/test/testdaemon.py | 1 + test-data/unit/daemon.test | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/mypy/test/testdaemon.py b/mypy/test/testdaemon.py index 73f60372cd74..3a55b3e01008 100644 --- a/mypy/test/testdaemon.py +++ b/mypy/test/testdaemon.py @@ -37,6 +37,7 @@ def test_daemon(testcase: DataDrivenTestCase) -> None: expected_lines = step[1:] assert cmd.startswith('$') cmd = cmd[1:].strip() + cmd = cmd.replace('{python}', sys.executable) sts, output = run_cmd(cmd) output_lines = output.splitlines() if sts: diff --git a/test-data/unit/daemon.test b/test-data/unit/daemon.test index 60f209c20b8b..27579556dfba 100644 --- a/test-data/unit/daemon.test +++ b/test-data/unit/daemon.test @@ -29,15 +29,15 @@ def f(): pass $ dmypy run -- foo.py --follow-imports=error Daemon started $ dmypy run -- foo.py --follow-imports=error -$ echo [mypy] >mypy.ini -$ echo disallow_untyped_defs = True >>mypy.ini +$ {python} -c "print('[mypy]')" >mypy.ini +$ {python} -c "print('disallow_untyped_defs = True')" >>mypy.ini $ dmypy run -- foo.py --follow-imports=error Restarting: configuration changed Daemon stopped Daemon started foo.py:1: error: Function is missing a type annotation == Return code: 1 -$ echo 'def f() -> None: pass' >foo.py +$ {python} -c "print('def f() -> None: pass')" >foo.py $ dmypy run -- foo.py --follow-imports=error $ dmypy stop Daemon stopped @@ -47,7 +47,7 @@ def f(): pass [case testDaemonRunRestartPluginVersion] $ dmypy run -- foo.py Daemon started -$ echo "" >> plug.py +$ {python} -c "print(' ')" >> plug.py $ dmypy run -- foo.py Restarting: plugins changed Daemon stopped From 959f89afcc0b24cee82b467085a8069f1665fe0c Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Tue, 20 Nov 2018 14:10:30 -0800 Subject: [PATCH 41/50] Remove busy wait and give better debug info --- mypy/ipc.py | 47 +++++++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/mypy/ipc.py b/mypy/ipc.py index 5d250de6182d..7c6d205459d7 100644 --- a/mypy/ipc.py +++ b/mypy/ipc.py @@ -95,31 +95,30 @@ def __init__(self, name: str, timeout: Optional[int]) -> None: super().__init__(name) if sys.platform == 'win32': timeout = timeout or 1000 # we need to set a timeout - while True: - try: - _winapi.WaitNamedPipe(self.name, timeout) - except WindowsError as e: - if e.winerror == _winapi.ERROR_SEM_TIMEOUT: - raise IPCException("Timed out waiting for connection.") - else: - raise - try: - self.connection = _winapi.CreateFile( - self.name, - _winapi.GENERIC_READ | _winapi.GENERIC_WRITE, - 0, - _winapi.NULL, - _winapi.OPEN_EXISTING, - 0, - _winapi.NULL, - ) - except WindowsError as e: - if e.winerror == _winapi.ERROR_PIPE_BUSY: - raise IPCException("The connection is busy.") - else: - raise + try: + _winapi.WaitNamedPipe(self.name, timeout) + except FileNotFoundError: + raise IPCException("The NamedPipe at {} was not found.".format(self.name)) + except WindowsError as e: + if e.winerror == _winapi.ERROR_SEM_TIMEOUT: + raise IPCException("Timed out waiting for connection.") + else: + raise + try: + self.connection = _winapi.CreateFile( + self.name, + _winapi.GENERIC_READ | _winapi.GENERIC_WRITE, + 0, + _winapi.NULL, + _winapi.OPEN_EXISTING, + 0, + _winapi.NULL, + ) + except WindowsError as e: + if e.winerror == _winapi.ERROR_PIPE_BUSY: + raise IPCException("The connection is busy.") else: - break + raise _winapi.SetNamedPipeHandleState(self.connection, _winapi.PIPE_READMODE_MESSAGE, None, From 326758c0c47b64ff17b1e423f9863b7282041ff9 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Tue, 20 Nov 2018 14:14:28 -0800 Subject: [PATCH 42/50] Temporary debug info --- mypy/dmypy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index f5f7f1d569aa..bfe557777d95 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -257,7 +257,7 @@ def do_status(args: argparse.Namespace) -> None: if args.verbose or 'error' in response: show_stats(response) if 'error' in response: - sys.exit("Daemon is stuck; consider %s kill" % sys.argv[0]) + sys.exit("Daemon is stuck with error %s; consider %s kill" % (response, sys.argv[0])) print("Daemon is up and running") From d8062829f4a502dc3f3d178849d653f61dd539fc Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Wed, 21 Nov 2018 18:03:18 -0800 Subject: [PATCH 43/50] Reuse NamedPipe for multiple connections --- mypy/ipc.py | 72 ++++++++++++++++++++++++++++++-------------- mypy/test/testipc.py | 24 +++++++++++++-- 2 files changed, 71 insertions(+), 25 deletions(-) diff --git a/mypy/ipc.py b/mypy/ipc.py index 7c6d205459d7..230039ed3feb 100644 --- a/mypy/ipc.py +++ b/mypy/ipc.py @@ -21,7 +21,13 @@ if sys.platform == 'win32': # This may be private, but it is needed for IPC on Windows, and is basically stable import _winapi + import ctypes + _IPCHandle = int + + kernel32 = ctypes.windll.kernel32 + DisconnectNamedPipe = kernel32.DisconnectNamedPipe # type: Callable[[_IPCHandle], int] + FlushFileBuffers = kernel32.FlushFileBuffers # type: Callable[[_IPCHandle], int] else: import socket _IPCHandle = socket.socket @@ -79,14 +85,6 @@ def close(self) -> None: else: self.connection.close() - def __exit__(self, - exc_ty: 'Optional[Type[BaseException]]' = None, - exc_val: Optional[BaseException] = None, - exc_tb: Optional[TracebackType] = None, - ) -> bool: - self.close() - return False - class IPCClient(IPCBase): """The client side of an IPC connection.""" @@ -131,6 +129,14 @@ def __init__(self, name: str, timeout: Optional[int]) -> None: def __enter__(self) -> 'IPCClient': return self + def __exit__(self, + exc_ty: 'Optional[Type[BaseException]]' = None, + exc_val: Optional[BaseException] = None, + exc_tb: Optional[TracebackType] = None, + ) -> bool: + self.close() + return False + class IPCServer(IPCBase): @@ -140,7 +146,24 @@ def __init__(self, name: str, timeout: Optional[int] = None) -> None: if sys.platform == 'win32': name = r'\\.\pipe\{}-{}.pipe'.format(name, os.getpid()) super().__init__(name) - if sys.platform != 'win32': + if sys.platform == 'win32': + self.connection = _winapi.CreateNamedPipe(self.name, + _winapi.PIPE_ACCESS_DUPLEX + | _winapi.FILE_FLAG_FIRST_PIPE_INSTANCE, + _winapi.PIPE_READMODE_MESSAGE + | _winapi.PIPE_TYPE_MESSAGE + | _winapi.PIPE_WAIT + | 0x8, # PIPE_REJECT_REMOTE_CLIENTS + 1, # one instance + self.BUFFER_SIZE, + self.BUFFER_SIZE, + 1000, # Default timeout in milis + 0, # Use default security descriptor + ) + if self.connection == -1: # INVALID_HANDLE_VALUE + err = _winapi.GetLastError() + raise IPCException('Invalid handle to pipe: {err}'.format(err)) + else: self.sock_directory = tempfile.mkdtemp() sockfile = os.path.join(self.sock_directory, self.name) self.sock = socket.socket(socket.AF_UNIX) @@ -151,19 +174,6 @@ def __init__(self, name: str, timeout: Optional[int] = None) -> None: def __enter__(self) -> 'IPCServer': if sys.platform == 'win32': - # The NamedPipe needs to be recreated every time we wait for a connection - self.connection = _winapi.CreateNamedPipe(self.name, - _winapi.PIPE_ACCESS_DUPLEX | _winapi.FILE_FLAG_FIRST_PIPE_INSTANCE, - _winapi.PIPE_READMODE_MESSAGE | _winapi.PIPE_TYPE_MESSAGE | _winapi.PIPE_WAIT, - 1, # one instance - self.BUFFER_SIZE, - self.BUFFER_SIZE, - 0, # PIPE_WAIT, wait for a connection of the client - 0, # PIPE_ACCEPT_REMOTE_CLIENTS (this is the default, so I guess?) - ) - if self.connection == -1: # INVALID_HANDLE_VALUE - err = _winapi.GetLastError() - raise IPCException('Invalid handle to pipe: {err}'.format(err)) # NOTE: It is theoretically possible that this will hang forever if the # client never connects, though this can be "solved" by killing the server try: @@ -178,8 +188,24 @@ def __enter__(self) -> 'IPCServer': raise IPCException('The socket timed out') return self + def __exit__(self, + exc_ty: 'Optional[Type[BaseException]]' = None, + exc_val: Optional[BaseException] = None, + exc_tb: Optional[TracebackType] = None, + ) -> bool: + if sys.platform == 'win32': + # Wait for the client to finish reading the last write before disconnecting + if not FlushFileBuffers(self.connection): + raise IPCException("Failed to flush NamedPipe buffer, maybe the client hung up?") + DisconnectNamedPipe(self.connection) + else: + self.close() + return False + def cleanup(self) -> None: - if sys.platform != 'win32': + if sys.platform == 'win32': + self.close() + else: shutil.rmtree(self.sock_directory) @property diff --git a/mypy/test/testipc.py b/mypy/test/testipc.py index 5ffbe49b1e40..660107b78325 100644 --- a/mypy/test/testipc.py +++ b/mypy/test/testipc.py @@ -13,8 +13,11 @@ def server(msg: str, q: 'Queue[str]') -> None: server = IPCServer(CONNECTION_NAME) q.put(server.connection_name) - with server: - server.write(msg.encode()) + data = b'' + while not data: + with server: + server.write(msg.encode()) + data = server.read() server.cleanup() @@ -27,6 +30,23 @@ def test_transaction_large(self) -> None: connection_name = queue.get() with IPCClient(connection_name, timeout=1) as client: assert client.read() == msg.encode() + client.write(b'test') + queue.close() + queue.join_thread() + p.join() + + def test_connect_twice(self) -> None: + queue = Queue() # type: Queue[str] + msg = 'this is a test message' + p = Process(target=server, args=(msg, queue), daemon=True) + p.start() + connection_name = queue.get() + with IPCClient(connection_name, timeout=1) as client: + assert client.read() == msg.encode() + client.write(b'') # don't let the server hang up yet, we want to connect again. + + with IPCClient(connection_name, timeout=1) as client: + client.write(b'test') queue.close() queue.join_thread() p.join() From 989ca7278d8256078fe34ad1a85b49630a2dbbd2 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Wed, 21 Nov 2018 18:25:51 -0800 Subject: [PATCH 44/50] Conditionally import Options in dmypy --- mypy/dmypy.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index bfe557777d95..2db7698aeb73 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -21,7 +21,7 @@ from mypy.dmypy_util import STATUS_FILE, receive from mypy.ipc import IPCClient, IPCException from mypy.dmypy_os import alive, kill -from mypy.options import Options + from mypy.version import __version__ # Argument parser. Subparsers are tied to action functions by the @@ -367,6 +367,7 @@ def do_daemon(args: argparse.Namespace) -> None: # Lazy import so this import doesn't slow down other commands. from mypy.dmypy_server import Server, process_start_options if args.options_data: + from mypy.options import Options options_dict, timeout, log_file = pickle.loads(base64.b64decode(args.options_data)) options_obj = Options() options = options_obj.apply_changes(options_dict) From da719d6bd148f0dc8db81f2ff1bf14f54f93f250 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Wed, 21 Nov 2018 19:04:23 -0800 Subject: [PATCH 45/50] Remove debug info --- mypy/dmypy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index 2db7698aeb73..619351f518a6 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -257,7 +257,7 @@ def do_status(args: argparse.Namespace) -> None: if args.verbose or 'error' in response: show_stats(response) if 'error' in response: - sys.exit("Daemon is stuck with error %s; consider %s kill" % (response, sys.argv[0])) + sys.exit("Daemon is stuck; consider %s kill" % (sys.argv[0])) print("Daemon is up and running") From a732a1e43a74ab70809f98659d7fb6bfae550baf Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Tue, 27 Nov 2018 14:27:05 -0800 Subject: [PATCH 46/50] Be more eager in removing status file --- mypy/dmypy_server.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 7b9a13f09254..5d22437fe1f8 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -217,10 +217,6 @@ def serve(self) -> None: reset_global_state() sys.exit(0) finally: - try: - server.cleanup() # try to remove the socket dir on Linux - except OSError: - pass # 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 @@ -228,6 +224,10 @@ def serve(self) -> None: # status file.) if command != 'stop': os.unlink(STATUS_FILE) + try: + server.cleanup() # try to remove the socket dir on Linux + except OSError: + pass exc_info = sys.exc_info() if exc_info[0] and exc_info[0] is not SystemExit: traceback.print_exception(*exc_info) From dae10eb0565e7b085a31573c630ba14bad7626eb Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Tue, 27 Nov 2018 14:27:45 -0800 Subject: [PATCH 47/50] Remove redundant parens --- mypy/dmypy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/dmypy.py b/mypy/dmypy.py index 619351f518a6..84af22ed65bd 100644 --- a/mypy/dmypy.py +++ b/mypy/dmypy.py @@ -257,7 +257,7 @@ def do_status(args: argparse.Namespace) -> None: if args.verbose or 'error' in response: show_stats(response) if 'error' in response: - sys.exit("Daemon is stuck; consider %s kill" % (sys.argv[0])) + sys.exit("Daemon is stuck; consider %s kill" % sys.argv[0]) print("Daemon is up and running") From 9de1b8887e2cb6da98c86cb75a839c7cad0a5f7a Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Tue, 27 Nov 2018 14:36:40 -0800 Subject: [PATCH 48/50] Use random bytes for NamedPipe name --- mypy/ipc.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mypy/ipc.py b/mypy/ipc.py index 230039ed3feb..5bc2c91f6ed1 100644 --- a/mypy/ipc.py +++ b/mypy/ipc.py @@ -4,6 +4,7 @@ On Windows, this uses NamedPipes. """ +import base64 import contextlib import os import shutil @@ -144,7 +145,7 @@ class IPCServer(IPCBase): def __init__(self, name: str, timeout: Optional[int] = None) -> None: if sys.platform == 'win32': - name = r'\\.\pipe\{}-{}.pipe'.format(name, os.getpid()) + name = r'\\.\pipe\{}-{}.pipe'.format(name, base64.b64encode(os.urandom(6))) super().__init__(name) if sys.platform == 'win32': self.connection = _winapi.CreateNamedPipe(self.name, From f3b8bb65ab50d62ec499a0745d419b4d60083dc9 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Tue, 27 Nov 2018 14:41:55 -0800 Subject: [PATCH 49/50] Document timeouts on Windows --- docs/source/mypy_daemon.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/source/mypy_daemon.rst b/docs/source/mypy_daemon.rst index 602d659d6fcf..aef94e8887ee 100644 --- a/docs/source/mypy_daemon.rst +++ b/docs/source/mypy_daemon.rst @@ -28,6 +28,12 @@ you'll find errors sooner. and it can only process one type checking request at a time. You can run multiple mypy daemon processes to type check multiple repositories. +.. note:: + On Windows, due to platform limitations, the mypy daemon does not currently + support a timeout for the server process. The client will still time out if + a connection to the server cannot be made, but the server will wait forever + for a new client connection. + Basic usage *********** From 0c43879ef6161d6f7cec7252e645ab0920115fe7 Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Tue, 27 Nov 2018 14:44:26 -0800 Subject: [PATCH 50/50] Add line in note --- docs/source/mypy_daemon.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/source/mypy_daemon.rst b/docs/source/mypy_daemon.rst index aef94e8887ee..17fa9e757bb7 100644 --- a/docs/source/mypy_daemon.rst +++ b/docs/source/mypy_daemon.rst @@ -29,6 +29,7 @@ you'll find errors sooner. run multiple mypy daemon processes to type check multiple repositories. .. note:: + On Windows, due to platform limitations, the mypy daemon does not currently support a timeout for the server process. The client will still time out if a connection to the server cannot be made, but the server will wait forever