Refactor port finding logic

This moves all the port logic into its own util function, which can be
tested without having to background a server directly.

spacy/displacy/__init__.py

@@ -11,7 +11,7 @@ from .render import DependencyRenderer, EntityRenderer, SpanRenderer
 from ..tokens import Doc, Span
 from ..errors import Errors, Warnings
 from ..util import is_in_jupyter
-from ..util import is_port_in_use
+from ..util import find_available_port
 
 
 _html = {}
@@ -102,36 +102,20 @@ def serve(
     """
     from wsgiref import simple_server
 
-    serve_port = port
+    port = find_available_port(port, host, auto_select_port)
-
-    if is_port_in_use(serve_port):
-        if not auto_select_port:
-            raise ValueError(Errors.E1049.format(port=port))
-
-        while is_port_in_use(serve_port) and serve_port < 65535:
-            serve_port += 1
 
     if is_in_jupyter():
         warnings.warn(Warnings.W011)
     render(
         docs, style=style, page=page, minify=minify, options=options, manual=manual
     )
-
+    httpd = simple_server.make_server(host, port, app)
-        if serve_port == 65535 and is_port_in_use(serve_port):
-            raise ValueError(Errors.E1048.format(host=host))
-
-        if serve_port != port:
-            warnings.warn(
-                Warnings.W124.format(host=host, port=port, serve_port=serve_port)
-            )
-
-    httpd = simple_server.make_server(host, serve_port, app)
     print(f"\nUsing the '{style}' visualizer")
-    print(f"Serving on http://{host}:{serve_port} ...\n")
+    print(f"Serving on http://{host}:{port} ...\n")
     try:
         httpd.serve_forever()
     except KeyboardInterrupt:
-        print(f"Shutting down server on port {serve_port}.")
+        print(f"Shutting down server on port {port}.")
     finally:
         httpd.server_close()
 

spacy/tests/test_displacy.py

@@ -1,5 +1,3 @@
-import multiprocessing
-import time
 import numpy
 import pytest
 
@@ -365,30 +363,3 @@ def test_displacy_manual_sorted_entities():
 
     html = displacy.render(doc, style="ent", manual=True)
     assert html.find("FIRST") < html.find("SECOND")
-
-
-def test_autoport(en_vocab):
-    """Test that automatic port selection works"""
-    doc = Doc(en_vocab, words=["Welcome", "to", "the", "Bank", "of", "China"])
-
-    def background_server():
-        displacy.serve([doc], auto_select_port=True)
-
-    proc1 = multiprocessing.Process(target=background_server)
-    proc2 = multiprocessing.Process(target=background_server)
-    try:
-        proc1.start()
-        assert proc1.is_alive(), "displaCy server didn't start"
-        proc2.start()
-        time.sleep(5)
-        assert proc2.is_alive(), "Second displaCy server didn't start"
-        proc1.terminate()
-        proc2.terminate()
-    finally:
-        if proc1.is_alive():
-            proc1.terminate()
-        if proc2.is_alive():
-            proc2.terminate()
-        time.sleep(2)
-        proc1.close()
-        proc2.close()

spacy/tests/test_misc.py

@@ -8,7 +8,7 @@ from spacy import prefer_gpu, require_gpu, require_cpu
 from spacy.ml._precomputable_affine import PrecomputableAffine
 from spacy.ml._precomputable_affine import _backprop_precomputable_affine_padding
 from spacy.util import dot_to_object, SimpleFrozenList, import_file
-from spacy.util import to_ternary_int
+from spacy.util import to_ternary_int, find_available_port
 from thinc.api import Config, Optimizer, ConfigValidationError
 from thinc.api import get_current_ops, set_current_ops, NumpyOps, CupyOps, MPSOps
 from thinc.compat import has_cupy_gpu, has_torch_mps_gpu
@@ -434,3 +434,16 @@ def test_to_ternary_int():
     assert to_ternary_int(-10) == -1
     assert to_ternary_int("string") == -1
     assert to_ternary_int([0, "string"]) == -1
+
+
+def test_find_available_port():
+    host = "0.0.0.0"
+    port = 5000
+    assert find_available_port(port, host) == port, "Port 5000 isn't free"
+
+    from wsgiref.simple_server import make_server, demo_app
+
+    httpd = make_server(host, port, demo_app)
+    with pytest.warns(UserWarning, match="already in use"):
+        found_port = find_available_port(port, host, auto_select_port=True)
+    assert found_port == port + 1, "Didn't find next port"

spacy/util.py

@@ -1749,3 +1749,30 @@ def is_port_in_use(port):
         return True
     finally:
         s.close()
+
+def find_available_port(start, host, auto_select_port=False):
+    """Given a starting port and a host, handle finding a port.
+
+    If `auto_select_port` is False, a busy port will raise an error.
+
+    If `auto_select_port` is True, the next free higher port will be used.
+    """
+    if not is_port_in_use(start):
+        return start
+
+    port = start
+    if not auto_select_port:
+        raise ValueError(Errors.E1049.format(port=port))
+
+    while is_port_in_use(port) and port < 65535:
+        port += 1
+
+    if port == 65535 and is_port_in_use(port):
+        raise ValueError(Errors.E1048.format(host=host))
+
+    # if we get here, the port changed
+    warnings.warn(
+        Warnings.W124.format(host=host, port=start, serve_port=port)
+    )
+    return port
+