From b4f0bf56333cd05144697ad157a66207fc108981 Mon Sep 17 00:00:00 2001 From: Nick Felt Date: Thu, 20 Sep 2018 15:07:24 -0700 Subject: [PATCH 1/6] Fix bug: expanding ~ for event_file but not using it --- tensorboard/program.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tensorboard/program.py b/tensorboard/program.py index 425945b9155..34e55562056 100644 --- a/tensorboard/program.py +++ b/tensorboard/program.py @@ -187,9 +187,7 @@ def main(self, ignored_argv=('',)): if self.flags.inspect: logger.info('Not bringing up TensorBoard, but inspecting event files.') event_file = os.path.expanduser(self.flags.event_file) - efi.inspect(self.flags.logdir, - self.flags.event_file, - self.flags.tag) + efi.inspect(self.flags.logdir, event_file, self.flags.tag) return 0 try: server = self._make_server() From 7af8c0889b5bb96cc6488e91fd987a19aa1b6893 Mon Sep 17 00:00:00 2001 From: Nick Felt Date: Thu, 20 Sep 2018 15:08:05 -0700 Subject: [PATCH 2/6] Rewrite WerkzeugServer for better IPV4/IPV6 handling logic --- tensorboard/program.py | 135 +++++++++++++++++++++++++++-------------- 1 file changed, 88 insertions(+), 47 deletions(-) diff --git a/tensorboard/program.py b/tensorboard/program.py index 34e55562056..2f5b21a172e 100644 --- a/tensorboard/program.py +++ b/tensorboard/program.py @@ -32,6 +32,7 @@ from abc import ABCMeta from abc import abstractmethod import argparse +from collections import defaultdict import errno import logging import os @@ -264,61 +265,101 @@ def __init__(self, msg): self.msg = msg -class WerkzeugServer(TensorBoardServer): +class WerkzeugServer(serving.ThreadedWSGIServer, TensorBoardServer): """Implementation of TensorBoardServer using the Werkzeug dev server.""" + # ThreadedWSGIServer handles this in werkzeug 0.12+ but we allow 0.11.x. + daemon_threads = True def __init__(self, wsgi_app, flags): + self._flags = flags host = flags.host - port = flags.port + self._auto_wildcard = False + if not host: + # Without an explicit host, we default to serving on all interfaces, + # and will attempt to serve both IPv4 and IPv6 traffic through one socket. + host = self._get_wildcard_address(flags.port) + self._auto_wildcard = True try: - if host: - # The user gave us an explicit host - server = serving.make_server(host, port, wsgi_app, threaded=True) - if ':' in host and not host.startswith('['): - # Display IPv6 addresses as [::1]:80 rather than ::1:80 - final_host = '[{}]'.format(host) + super(WerkzeugServer, self).__init__(host, flags.port, wsgi_app) + except socket.error as e: + if hasattr(errno, 'EACCES') and e.errno == errno.EACCES: + msg = ('TensorBoard must be run as superuser to bind to port %d' + % flags.port) + elif hasattr(errno, 'EADDRINUSE') and e.errno == errno.EADDRINUSE: + if flags.port == 0: + msg = 'TensorBoard unable to find any open port' else: - final_host = host + msg = ('TensorBoard could not bind to port %d, it was already in use' + % flags.port) else: - # We've promised to bind to all interfaces on this host. However, we're - # not sure whether that means IPv4 or IPv6 interfaces. - try: - # First try passing in a blank host (meaning all interfaces). This, - # unfortunately, defaults to IPv4 even if no IPv4 interface is available - # (yielding a socket.error). - server = serving.make_server(host, port, wsgi_app, threaded=True) - except socket.error: - # If a blank host didn't work, we explicitly request IPv6 interfaces. - server = serving.make_server('::', port, wsgi_app, threaded=True) - final_host = socket.gethostname() - server.daemon_threads = True - except socket.error: - if port == 0: - msg = 'TensorBoard unable to find any open port' - else: - msg = ( - 'TensorBoard attempted to bind to port %d, but it was already in use' - % port) + raise raise TensorBoardServerException(msg) - server.handle_error = _handle_error - final_port = server.socket.getsockname()[1] - self._server = server - self._url = 'http://%s:%d%s' % (final_host, final_port, flags.path_prefix) - def serve_forever(self): - self._server.serve_forever() + def _get_wildcard_address(self, port): + """Returns a wildcard address for the port in question. + + This will attempt to follow the best practice of calling getaddrinfo() with + a null host and AI_PASSIVE to request a server-side socket wilcard address. + If that succeeds, this returns the first IPv6 address found, or if none, + then returns the first IPv4 address. If that fails, then this returns the + hardcoded address "::" if socket.has_ipv6 is True, else "0.0.0.0". + """ + fallback_address = '::' if socket.has_ipv6 else '0.0.0.0' + if hasattr(socket, 'AI_PASSIVE'): + try: + wildcard_addrs = socket.getaddrinfo( + None, port, socket.AF_UNSPEC, socket.SOCK_STREAM, + socket.IPPROTO_TCP, socket.AI_PASSIVE) + except socket.gaierror as e: + logger.warn('Failed to auto-detect wildcard address, assuming %s: %s', + fallback_address, str(e)) + return fallback_address + addrs_by_family = defaultdict(list) + for family, _, _, _, sockaddr in wildcard_addrs: + # Format of sockaddr varies by family, but [0] is always the address. + addrs_by_family[family].append(sockaddr[0]) + if hasattr(socket, 'AF_INET6') and addrs_by_family[socket.AF_INET6]: + return addrs_by_family[socket.AF_INET6][0] + if hasattr(socket, 'AF_INET') and addrs_by_family[socket.AF_INET]: + return addrs_by_family[socket.AF_INET][0] + logger.warn('Failed to auto-detect wildcard address, assuming %s', + fallback_address) + return fallback_address + + def server_bind(self): + """Override to enable IPV4 mapping for IPV6 sockets when desired. + + The main use case for this is so that when no host is specified, TensorBoard + can listen on all interfaces for both IPv4 and IPv6 connections, rather than + having to choose v4 or v6 and hope the browser didn't choose the other one. + """ + socket_is_v6 = ( + hasattr(socket, 'AF_INET6') and self.socket.family == socket.AF_INET6) + has_v6only_option = ( + hasattr(socket, 'IPPROTO_IPV6') and hasattr(socket, 'IPV6_V6ONLY')) + if self._auto_wildcard and socket_is_v6 and has_v6only_option: + self.socket.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_V6ONLY, 0) + super(WerkzeugServer, self).server_bind() + + def handle_error(self, request, client_address): + """Override to get rid of noisy EPIPE errors.""" + del request # unused + # Kludge to override a SocketServer.py method so we can get rid of noisy + # EPIPE errors. They're kind of a red herring as far as errors go. For + # example, `curl -N http://localhost:6006/ | head` will cause an EPIPE. + exc_info = sys.exc_info() + e = exc_info[1] + if isinstance(e, IOError) and e.errno == errno.EPIPE: + logger.warn('EPIPE caused by %s in HTTP serving' % str(client_address)) + else: + logger.error('HTTP serving error', exc_info=exc_info) def get_url(self): - return self._url - - -# Kludge to override a SocketServer.py method so we can get rid of noisy -# EPIPE errors. They're kind of a red herring as far as errors go. For -# example, `curl -N http://localhost:6006/ | head` will cause an EPIPE. -def _handle_error(unused_request, client_address): - exc_info = sys.exc_info() - e = exc_info[1] - if isinstance(e, IOError) and e.errno == errno.EPIPE: - logger.warn('EPIPE caused by %s in HTTP serving' % client_address) - else: - logger.error('HTTP serving error', exc_info=exc_info) + if self._auto_wildcard: + display_host = socket.gethostname() + else: + host = self._flags.host + display_host = ( + '[%s]' % host if ':' in host and not host.startswith('[') else host) + return 'http://%s:%d%s' % (display_host, self.server_port, + self._flags.path_prefix) From 8f8177bb770cffcd1b802037b0c94794918a35fc Mon Sep 17 00:00:00 2001 From: Nick Felt Date: Thu, 20 Sep 2018 16:18:38 -0700 Subject: [PATCH 3/6] typo fix --- tensorboard/program.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tensorboard/program.py b/tensorboard/program.py index 2f5b21a172e..311d9baa107 100644 --- a/tensorboard/program.py +++ b/tensorboard/program.py @@ -299,7 +299,7 @@ def _get_wildcard_address(self, port): """Returns a wildcard address for the port in question. This will attempt to follow the best practice of calling getaddrinfo() with - a null host and AI_PASSIVE to request a server-side socket wilcard address. + a null host and AI_PASSIVE to request a server-side socket wildcard address. If that succeeds, this returns the first IPv6 address found, or if none, then returns the first IPv4 address. If that fails, then this returns the hardcoded address "::" if socket.has_ipv6 is True, else "0.0.0.0". From a3281ba93f605e343cab9e454308ad00bd474c17 Mon Sep 17 00:00:00 2001 From: Nick Felt Date: Fri, 21 Sep 2018 13:24:34 -0700 Subject: [PATCH 4/6] review: simplify control flow --- tensorboard/program.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tensorboard/program.py b/tensorboard/program.py index 311d9baa107..34a452190be 100644 --- a/tensorboard/program.py +++ b/tensorboard/program.py @@ -283,17 +283,19 @@ def __init__(self, wsgi_app, flags): super(WerkzeugServer, self).__init__(host, flags.port, wsgi_app) except socket.error as e: if hasattr(errno, 'EACCES') and e.errno == errno.EACCES: - msg = ('TensorBoard must be run as superuser to bind to port %d' - % flags.port) + raise TensorBoardServerException( + 'TensorBoard must be run as superuser to bind to port %d' % + flags.port) elif hasattr(errno, 'EADDRINUSE') and e.errno == errno.EADDRINUSE: if flags.port == 0: - msg = 'TensorBoard unable to find any open port' + raise TensorBoardServerException( + 'TensorBoard unable to find any open port') else: - msg = ('TensorBoard could not bind to port %d, it was already in use' - % flags.port) - else: - raise - raise TensorBoardServerException(msg) + raise TensorBoardServerException( + 'TensorBoard could not bind to port %d, it was already in use' % + flags.port) + # Raise the raw exception if it wasn't identifiable as a user error. + raise def _get_wildcard_address(self, port): """Returns a wildcard address for the port in question. From 9f3af189002e7d3b1f6ab80c5c76c8e6507a59f2 Mon Sep 17 00:00:00 2001 From: Nick Felt Date: Fri, 21 Sep 2018 13:24:46 -0700 Subject: [PATCH 5/6] review: clarify address naming --- tensorboard/program.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tensorboard/program.py b/tensorboard/program.py index 34a452190be..1d4479708f5 100644 --- a/tensorboard/program.py +++ b/tensorboard/program.py @@ -309,16 +309,17 @@ def _get_wildcard_address(self, port): fallback_address = '::' if socket.has_ipv6 else '0.0.0.0' if hasattr(socket, 'AI_PASSIVE'): try: - wildcard_addrs = socket.getaddrinfo( - None, port, socket.AF_UNSPEC, socket.SOCK_STREAM, - socket.IPPROTO_TCP, socket.AI_PASSIVE) + addrinfos = socket.getaddrinfo(None, port, socket.AF_UNSPEC, + socket.SOCK_STREAM, socket.IPPROTO_TCP, + socket.AI_PASSIVE) except socket.gaierror as e: logger.warn('Failed to auto-detect wildcard address, assuming %s: %s', fallback_address, str(e)) return fallback_address addrs_by_family = defaultdict(list) - for family, _, _, _, sockaddr in wildcard_addrs: - # Format of sockaddr varies by family, but [0] is always the address. + for family, _, _, _, sockaddr in addrinfos: + # Format of the "sockaddr" socket address varies by address family, + # but [0] is always the IP address portion. addrs_by_family[family].append(sockaddr[0]) if hasattr(socket, 'AF_INET6') and addrs_by_family[socket.AF_INET6]: return addrs_by_family[socket.AF_INET6][0] From 2dc34770ba8ec672d4775fcba7aafb4a6543fd31 Mon Sep 17 00:00:00 2001 From: Nick Felt Date: Fri, 21 Sep 2018 13:24:51 -0700 Subject: [PATCH 6/6] add better error handling for v4-only and v6-only stacks --- tensorboard/program.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tensorboard/program.py b/tensorboard/program.py index 1d4479708f5..d7905c8938d 100644 --- a/tensorboard/program.py +++ b/tensorboard/program.py @@ -294,6 +294,13 @@ def __init__(self, wsgi_app, flags): raise TensorBoardServerException( 'TensorBoard could not bind to port %d, it was already in use' % flags.port) + elif hasattr(errno, 'EADDRNOTAVAIL') and e.errno == errno.EADDRNOTAVAIL: + raise TensorBoardServerException( + 'TensorBoard could not bind to unavailable address %s' % host) + elif hasattr(errno, 'EAFNOSUPPORT') and e.errno == errno.EAFNOSUPPORT: + raise TensorBoardServerException( + 'Tensorboard could not bind to unsupported address family %s' % + host) # Raise the raw exception if it wasn't identifiable as a user error. raise @@ -341,7 +348,13 @@ def server_bind(self): has_v6only_option = ( hasattr(socket, 'IPPROTO_IPV6') and hasattr(socket, 'IPV6_V6ONLY')) if self._auto_wildcard and socket_is_v6 and has_v6only_option: - self.socket.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_V6ONLY, 0) + try: + self.socket.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_V6ONLY, 0) + except socket.error as e: + # Log a warning on failure to dual-bind, except for EAFNOSUPPORT + # since that's expected if IPv4 isn't supported at all (IPv6-only). + if hasattr(errno, 'EAFNOSUPPORT') and e.errno != errno.EAFNOSUPPORT: + logging.warn('Failed to dual-bind to IPv4 wildcard: %s', str(e)) super(WerkzeugServer, self).server_bind() def handle_error(self, request, client_address):