From 95414238f34a85b2b137e0b3e3119324e065519c Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Mon, 10 Sep 2012 09:53:08 -0700 Subject: [PATCH] Replace get_unused_port with bind_unused_port, which binds to port 0 to allow the OS to select a port for us. Closes #590. --- tornado/test/httpclient_test.py | 6 ++---- tornado/test/ioloop_test.py | 7 ++----- tornado/test/iostream_test.py | 9 ++++---- tornado/test/process_test.py | 14 +++++-------- tornado/test/simple_httpclient_test.py | 5 +++-- tornado/test/twisted_test.py | 12 ++++++----- tornado/testing.py | 29 ++++++++++++++++++++++---- website/sphinx/releases/next.rst | 3 +++ website/sphinx/testing.rst | 2 ++ 9 files changed, 53 insertions(+), 34 deletions(-) diff --git a/tornado/test/httpclient_test.py b/tornado/test/httpclient_test.py index 08afff08..54ebc749 100644 --- a/tornado/test/httpclient_test.py +++ b/tornado/test/httpclient_test.py @@ -8,10 +8,9 @@ from contextlib import closing import functools from tornado.escape import utf8 -from tornado.httpclient import AsyncHTTPClient from tornado.iostream import IOStream from tornado import netutil -from tornado.testing import AsyncHTTPTestCase, get_unused_port +from tornado.testing import AsyncHTTPTestCase, bind_unused_port from tornado.util import b, bytes_type from tornado.web import Application, RequestHandler, url @@ -108,8 +107,7 @@ class HTTPClientCommonTestCase(AsyncHTTPTestCase): def test_chunked_close(self): # test case in which chunks spread read-callback processing # over several ioloop iterations, but the connection is already closed. - port = get_unused_port() - (sock,) = netutil.bind_sockets(port, address="127.0.0.1") + sock, port = bind_unused_port() with closing(sock): def write_response(stream, request_data): stream.write(b("""\ diff --git a/tornado/test/ioloop_test.py b/tornado/test/ioloop_test.py index 63ef31f5..a679b07f 100644 --- a/tornado/test/ioloop_test.py +++ b/tornado/test/ioloop_test.py @@ -3,12 +3,10 @@ from __future__ import absolute_import, division, with_statement import datetime -import socket import time from tornado.ioloop import IOLoop -from tornado.netutil import bind_sockets -from tornado.testing import AsyncTestCase, get_unused_port +from tornado.testing import AsyncTestCase, bind_unused_port from tornado.test.util import unittest @@ -35,8 +33,7 @@ class TestIOLoop(AsyncTestCase): self.wait() def test_multiple_add(self): - [sock] = bind_sockets(get_unused_port(), '127.0.0.1', - family=socket.AF_INET) + sock, port = bind_unused_port() try: self.io_loop.add_handler(sock.fileno(), lambda fd, events: None, IOLoop.READ) diff --git a/tornado/test/iostream_test.py b/tornado/test/iostream_test.py index 0a4c1258..d06c69b3 100644 --- a/tornado/test/iostream_test.py +++ b/tornado/test/iostream_test.py @@ -3,7 +3,7 @@ from tornado import netutil from tornado.ioloop import IOLoop from tornado.iostream import IOStream, SSLIOStream from tornado.log import gen_log -from tornado.testing import AsyncHTTPTestCase, AsyncHTTPSTestCase, AsyncTestCase, get_unused_port, ExpectLog +from tornado.testing import AsyncHTTPTestCase, AsyncHTTPSTestCase, AsyncTestCase, bind_unused_port, ExpectLog from tornado.test.util import unittest from tornado.util import b from tornado.web import RequestHandler, Application @@ -120,9 +120,7 @@ class TestIOStreamMixin(object): raise NotImplementedError() def make_iostream_pair(self, **kwargs): - port = get_unused_port() - [listener] = netutil.bind_sockets(port, '127.0.0.1', - family=socket.AF_INET) + listener, port = bind_unused_port() streams = [None, None] def accept_callback(connection, address): @@ -168,7 +166,8 @@ class TestIOStreamMixin(object): # When a connection is refused, the connect callback should not # be run. (The kqueue IOLoop used to behave differently from the # epoll IOLoop in this respect) - port = get_unused_port() + server_socket, port = bind_unused_port() + server_socket.close() stream = IOStream(socket.socket(), self.io_loop) self.connect_called = False diff --git a/tornado/test/process_test.py b/tornado/test/process_test.py index 69ac6847..d076cac4 100644 --- a/tornado/test/process_test.py +++ b/tornado/test/process_test.py @@ -10,10 +10,9 @@ from tornado.httpclient import HTTPClient, HTTPError from tornado.httpserver import HTTPServer from tornado.ioloop import IOLoop from tornado.log import gen_log -from tornado.netutil import bind_sockets from tornado.process import fork_processes, task_id from tornado.simple_httpclient import SimpleAsyncHTTPClient -from tornado.testing import get_unused_port, ExpectLog +from tornado.testing import bind_unused_port, ExpectLog from tornado.test.util import unittest from tornado.web import RequestHandler, Application @@ -50,11 +49,10 @@ class ProcessTest(unittest.TestCase): def test_multi_process(self): with ExpectLog(gen_log, "(Starting .* processes|child .* exited|uncaught exception)"): self.assertFalse(IOLoop.initialized()) - port = get_unused_port() + sock, port = bind_unused_port() def get_url(path): return "http://127.0.0.1:%d%s" % (port, path) - sockets = bind_sockets(port, "127.0.0.1") # ensure that none of these processes live too long signal.alarm(5) # master process try: @@ -66,19 +64,17 @@ class ProcessTest(unittest.TestCase): # finished with status 0 self.assertEqual(e.code, 0) self.assertTrue(task_id() is None) - for sock in sockets: - sock.close() + sock.close() return try: if id in (0, 1): self.assertEqual(id, task_id()) server = HTTPServer(self.get_app()) - server.add_sockets(sockets) + server.add_sockets([sock]) IOLoop.instance().start() elif id == 2: self.assertEqual(id, task_id()) - for sock in sockets: - sock.close() + sock.close() # Always use SimpleAsyncHTTPClient here; the curl # version appears to get confused sometimes if the # connection gets closed before it's had a chance to diff --git a/tornado/test/simple_httpclient_test.py b/tornado/test/simple_httpclient_test.py index 05a35066..dbd836a3 100644 --- a/tornado/test/simple_httpclient_test.py +++ b/tornado/test/simple_httpclient_test.py @@ -17,7 +17,7 @@ from tornado.log import gen_log from tornado.simple_httpclient import SimpleAsyncHTTPClient, _DEFAULT_CA_CERTS from tornado.test.httpclient_test import ChunkHandler, CountdownHandler, HelloWorldHandler from tornado.test import httpclient_test -from tornado.testing import AsyncHTTPTestCase, AsyncTestCase, get_unused_port, ExpectLog +from tornado.testing import AsyncHTTPTestCase, AsyncTestCase, bind_unused_port, ExpectLog from tornado.test.util import unittest from tornado.util import b from tornado.web import RequestHandler, Application, asynchronous, url @@ -288,7 +288,8 @@ class SimpleHTTPClientTestCase(AsyncHTTPTestCase): self.assertTrue(host_re.match(response.body), response.body) def test_connection_refused(self): - port = get_unused_port() + server_socket, port = bind_unused_port() + server_socket.close() with ExpectLog(gen_log, ".*"): self.http_client.fetch("http://localhost:%d/" % port, self.stop) response = self.wait() diff --git a/tornado/test/twisted_test.py b/tornado/test/twisted_test.py index 997dc129..7521d740 100644 --- a/tornado/test/twisted_test.py +++ b/tornado/test/twisted_test.py @@ -42,9 +42,10 @@ except ImportError: have_twisted = False from tornado.httpclient import AsyncHTTPClient +from tornado.httpserver import HTTPServer from tornado.ioloop import IOLoop from tornado.platform.auto import set_close_exec -from tornado.testing import get_unused_port +from tornado.testing import bind_unused_port from tornado.test.util import unittest from tornado.util import import_object from tornado.web import RequestHandler, Application @@ -344,8 +345,8 @@ class CompatibilityTests(unittest.TestCase): def render_GET(self, request): return "Hello from twisted!" site = Site(HelloResource()) - self.twisted_port = get_unused_port() - self.reactor.listenTCP(self.twisted_port, site, interface='127.0.0.1') + port = self.reactor.listenTCP(0, site, interface='127.0.0.1') + self.twisted_port = port.getHost().port def start_tornado_server(self): class HelloHandler(RequestHandler): @@ -353,8 +354,9 @@ class CompatibilityTests(unittest.TestCase): self.write("Hello from tornado!") app = Application([('/', HelloHandler)], log_function=lambda x: None) - self.tornado_port = get_unused_port() - app.listen(self.tornado_port, address='127.0.0.1', io_loop=self.io_loop) + server = HTTPServer(app, io_loop=self.io_loop) + sock, self.tornado_port = bind_unused_port() + server.add_sockets([sock]) def run_ioloop(self): self.stop_loop = self.io_loop.stop diff --git a/tornado/testing.py b/tornado/testing.py index be507d38..5b529a26 100644 --- a/tornado/testing.py +++ b/tornado/testing.py @@ -26,12 +26,14 @@ try: from tornado.httpserver import HTTPServer from tornado.simple_httpclient import SimpleAsyncHTTPClient from tornado.ioloop import IOLoop + from tornado import netutil except ImportError: # These modules are not importable on app engine. Parts of this module # won't work, but e.g. LogTrapTestCase and main() will. AsyncHTTPClient = None HTTPServer = None IOLoop = None + netutil = None SimpleAsyncHTTPClient = None from tornado.log import gen_log from tornado.stack_context import StackContext, NullContext @@ -41,6 +43,7 @@ import logging import os import re import signal +import socket import sys import time @@ -57,13 +60,31 @@ _next_port = 10000 def get_unused_port(): - """Returns a (hopefully) unused port number.""" + """Returns a (hopefully) unused port number. + + This function does not guarantee that the port it returns is available, + only that a series of get_unused_port calls in a single process return + distinct ports. + + **Deprecated**. Use bind_unused_port instead, which is guaranteed + to find an unused port. + """ global _next_port port = _next_port _next_port = _next_port + 1 return port +def bind_unused_port(): + """Binds a server socket to an available port on localhost. + + Returns a tuple (socket, port). + """ + [sock] = netutil.bind_sockets(0, 'localhost', family=socket.AF_INET) + port = sock.getsockname()[1] + return sock, port + + class AsyncTestCase(unittest.TestCase): """TestCase subclass for testing IOLoop-based asynchronous code. @@ -248,7 +269,9 @@ class AsyncHTTPTestCase(AsyncTestCase): self.http_client = self.get_http_client() self._app = self.get_app() self.http_server = self.get_http_server() - self.http_server.listen(self.get_http_port(), address="127.0.0.1") + sock, port = bind_unused_port() + self.http_server.add_sockets([sock]) + self.__port = port def get_http_client(self): return AsyncHTTPClient(io_loop=self.io_loop) @@ -285,8 +308,6 @@ class AsyncHTTPTestCase(AsyncTestCase): A new port is chosen for each test. """ - if self.__port is None: - self.__port = get_unused_port() return self.__port def get_protocol(self): diff --git a/website/sphinx/releases/next.rst b/website/sphinx/releases/next.rst index f4bd8cc0..e5f7ed04 100644 --- a/website/sphinx/releases/next.rst +++ b/website/sphinx/releases/next.rst @@ -30,3 +30,6 @@ In progress * Empty HTTP request arguments are no longer ignored. This applies to ``HTTPRequest.arguments`` and ``RequestHandler.get_argument[s]`` in WSGI and non-WSGI modes. +* New function `tornado.testing.bind_unused_port` both chooses a port + and binds a socket to it, so there is no risk of another process + using the same port. ``get_unused_port`` is now deprecated. diff --git a/website/sphinx/testing.rst b/website/sphinx/testing.rst index faf4ac7a..24f5c926 100644 --- a/website/sphinx/testing.rst +++ b/website/sphinx/testing.rst @@ -32,4 +32,6 @@ Helper functions ---------------- + .. autofunction:: bind_unused_port + .. autofunction:: get_unused_port