Bootstrap Updates

src/ffpuppet/bootstrapper.py

@@ -2,8 +2,13 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 """ffpuppet bootstrapper module"""
-import socket
+
+# as of python 3.10 socket.timeout was made an alias of TimeoutError
+# pylint: disable=ungrouped-imports
+from socket import timeout as socket_timeout  # isort: skip
+
 from logging import getLogger
+from socket import SO_REUSEADDR, SOL_SOCKET, socket
 from time import sleep, time
 from typing import Any, Callable, Optional
 
@@ -17,8 +22,8 @@
 
 class Bootstrapper:  # pylint: disable=missing-docstring
     # see: searchfox.org/mozilla-central/source/netwerk/base/nsIOService.cpp
-    # include ports above 1024
-    BLOCKED_PORTS = (
+    # include ports above 1023
+    BLOCKED_PORTS = {
         1719,
         1720,
         1723,
@@ -36,39 +41,16 @@ class Bootstrapper:  # pylint: disable=missing-docstring
         6669,
         6697,
         10080,
-    )
+    }
     # receive buffer size
     BUF_SIZE = 4096
     # duration of initial blocking socket operations
-    POLL_WAIT: float = 1
-    # number of attempts to find an available port
-    PORT_ATTEMPTS = 50
+    POLL_WAIT = 1.0
 
     __slots__ = ("_socket",)
 
-    def __init__(self, attempts: int = PORT_ATTEMPTS, port: int = 0) -> None:
-        assert attempts > 0
-        assert port >= 0
-        for _ in range(attempts):
-            self._socket: Optional[socket.socket] = socket.socket()
-            self._socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
-            self._socket.settimeout(self.POLL_WAIT)
-            try:
-                self._socket.bind(("127.0.0.1", port))
-                self._socket.listen(5)
-            except (OSError, PermissionError) as exc:
-                LOG.debug("%s: %s", type(exc).__name__, exc)
-                self._socket.close()
-                sleep(0.1)
-                continue
-            # avoid blocked ports
-            if port == 0 and self._socket.getsockname()[1] in self.BLOCKED_PORTS:
-                LOG.debug("bound to blocked port, retrying...")
-                self._socket.close()
-                continue
-            break
-        else:
-            raise LaunchError("Could not find available port")
+    def __init__(self, sock: socket) -> None:
+        self._socket = sock
 
     def __enter__(self) -> "Bootstrapper":
         return self
@@ -85,9 +67,42 @@ def close(self) -> None:
         Returns:
             None
         """
-        if self._socket is not None:
-            self._socket.close()
-            self._socket = None
+        self._socket.close()
+
+    @classmethod
+    def create(cls, attempts: int = 50, port: int = 0) -> "Bootstrapper":
+        """Create a Bootstrapper.
+
+        Args:
+            attempts: Number of times to attempt to bind.
+            port: Port to use. Use 0 for system select.
+
+        Returns:
+            Bootstrapper.
+        """
+        assert attempts > 0
+        assert port == 0 or 1024 <= port <= 65535
+        assert port not in cls.BLOCKED_PORTS
+        for _ in range(attempts):
+            sock = socket()
+            sock.setsockopt(SOL_SOCKET, SO_REUSEADDR, 1)
+            try:
+                sock.bind(("127.0.0.1", port))
+                sock.listen()
+            except (OSError, PermissionError) as exc:
+                LOG.debug("%s: %s", type(exc).__name__, exc)
+                sock.close()
+                sleep(0.1)
+                continue
+            # avoid blocked ports
+            if sock.getsockname()[1] in cls.BLOCKED_PORTS:
+                LOG.debug("bound to blocked port, retrying...")
+                sock.close()
+                continue
+            break
+        else:
+            raise LaunchError("Could not find available port")
+        return cls(sock)
 
     @property
     def location(self) -> str:
@@ -99,7 +114,6 @@ def location(self) -> str:
         Returns:
             Location.
         """
-        assert self._socket is not None
         return f"http://127.0.0.1:{self.port}"
 
     @property
@@ -112,7 +126,6 @@ def port(self) -> int:
         Returns:
             Port number.
         """
-        assert self._socket is not None
         return int(self._socket.getsockname()[1])
 
     def wait(
@@ -124,26 +137,25 @@ def wait(
         """Wait for browser connection, read request and send response.
 
         Args:
-            cb_continue: Callback that return True if the browser
-                         process is healthy otherwise False.
+            cb_continue: Callback that communicates browser process health.
             timeout: Amount of time wait before raising BrowserTimeoutError.
             url: Location to redirect to.
 
         Returns:
             None
         """
-        assert self._socket is not None
         assert timeout >= 0
         start_time = time()
         time_limit = start_time + timeout
-        conn: Optional[socket.socket] = None
+        conn: Optional[socket] = None
         try:
+            self._socket.settimeout(self.POLL_WAIT)
             while conn is None:
                 LOG.debug("waiting for browser connection...")
                 while conn is None:
                     try:
                         conn, _ = self._socket.accept()
-                    except socket.timeout:
+                    except socket_timeout:
                         if not cb_continue():
                             raise BrowserTerminatedError(
                                 "Failure waiting for browser connection"
@@ -161,7 +173,7 @@ def wait(
                     try:
                         count_recv = len(conn.recv(self.BUF_SIZE))
                         total_recv += count_recv
-                    except socket.timeout:
+                    except socket_timeout:
                         # use -1 to indicate timeout
                         count_recv = -1
                     if count_recv == self.BUF_SIZE:
@@ -193,7 +205,7 @@ def wait(
             LOG.debug("sending response (redirect %r)", url)
             try:
                 conn.sendall(resp.encode("ascii"))
-            except socket.timeout:
+            except socket_timeout:
                 resp_timeout = True
             else:
                 resp_timeout = False

src/ffpuppet/core.py

@@ -498,7 +498,7 @@ def close(self, force_close: bool = False) -> None:
                 LOG.warning("Crash reports still open after 30s")
         elif self._proc_tree.is_running():
             r_code = Reason.CLOSED
-        elif self._proc_tree.wait() not in {0, -1, 1, -2, -9, 245}:
+        elif abs(self._proc_tree.wait()) not in {0, 1, 2, 9, 15, 245}:
             # Note: ignore 245 for now to avoid getting flooded with OOMs that don't
             # have a crash report... this should be revisited when time allows
             # https://bugzil.la/1370520
@@ -722,7 +722,7 @@ def launch(
 
         # performing the bootstrap helps guarantee that the browser
         # will be loaded and ready to accept input when launch() returns
-        bootstrapper = Bootstrapper()
+        bootstrapper = Bootstrapper.create()
         try:
             # added `network.proxy.failover_direct` and `network.proxy.allow_bypass`
             # to workaround default prefs.js packaged with Grizzly test cases.

src/ffpuppet/test_bootstrapper.py

@@ -5,8 +5,12 @@
 """ffpuppet bootstrapper tests"""
 # pylint: disable=protected-access
 
+# as of python 3.10 socket.timeout was made an alias of TimeoutError
+# pylint: disable=ungrouped-imports
+from socket import timeout as socket_timeout  # isort: skip
+
 from itertools import repeat
-from socket import socket, timeout
+from socket import socket
 from threading import Thread
 
 from pytest import mark, raises
@@ -16,23 +20,21 @@
 
 
 def test_bootstrapper_01():
-    """test simple Bootstrapper()"""
-    with Bootstrapper() as bts:
+    """test Bootstrapper.create()"""
+    with Bootstrapper.create() as bts:
         assert bts._socket is not None
         assert bts.location.startswith("http://127.0.0.1:")
-        assert int(bts.location.split(":")[-1]) > 1024
-        assert bts.port > 1024
+        assert int(bts.location.split(":")[-1]) >= 1024
+        assert bts.port >= 1024
         assert bts.port not in Bootstrapper.BLOCKED_PORTS
         bts.close()
-        assert bts._socket is None
 
 
 def test_bootstrapper_02(mocker):
     """test Bootstrapper.wait() failure waiting for initial connection"""
     fake_sock = mocker.MagicMock(spec_set=socket)
-    fake_sock.accept.side_effect = timeout
-    mocker.patch("ffpuppet.bootstrapper.socket.socket", return_value=fake_sock)
-    with Bootstrapper() as bts:
+    fake_sock.accept.side_effect = socket_timeout
+    with Bootstrapper(fake_sock) as bts:
         # test failure
         with raises(
             BrowserTerminatedError, match="Failure waiting for browser connection"
@@ -54,10 +56,9 @@ def test_bootstrapper_03(mocker):
     """test Bootstrapper.wait() failure waiting for request"""
     fake_sock = mocker.MagicMock(spec_set=socket)
     fake_conn = mocker.Mock(spec_set=socket)
-    fake_conn.recv.side_effect = timeout
+    fake_conn.recv.side_effect = socket_timeout
     fake_sock.accept.return_value = (fake_conn, None)
-    mocker.patch("ffpuppet.bootstrapper.socket.socket", return_value=fake_sock)
-    with Bootstrapper() as bts:
+    with Bootstrapper(fake_sock) as bts:
         # test failure
         with raises(BrowserTerminatedError, match="Failure waiting for request"):
             bts.wait(lambda: False)
@@ -78,10 +79,9 @@ def test_bootstrapper_04(mocker):
     fake_sock = mocker.MagicMock(spec_set=socket)
     fake_conn = mocker.Mock(spec_set=socket)
     fake_conn.recv.return_value = "A"
-    fake_conn.sendall.side_effect = timeout
+    fake_conn.sendall.side_effect = socket_timeout
     fake_sock.accept.return_value = (fake_conn, None)
-    mocker.patch("ffpuppet.bootstrapper.socket.socket", return_value=fake_sock)
-    with Bootstrapper() as bts:
+    with Bootstrapper(fake_sock) as bts:
         # test timeout
         with raises(BrowserTimeoutError, match="Timeout sending response"):
             bts.wait(lambda: True)
@@ -103,8 +103,7 @@ def test_bootstrapper_05(mocker):
     fake_conn = mocker.Mock(spec_set=socket)
     fake_conn.recv.return_value = "foo"
     fake_sock.accept.return_value = (fake_conn, None)
-    mocker.patch("ffpuppet.bootstrapper.socket.socket", return_value=fake_sock)
-    with Bootstrapper() as bts:
+    with Bootstrapper(fake_sock) as bts:
         with raises(BrowserTerminatedError, match="Failure during browser startup"):
             bts.wait(lambda: False)
     assert fake_conn.close.call_count == 1
@@ -118,13 +117,13 @@ def test_bootstrapper_05(mocker):
         # with a redirect url
         ("http://127.0.0.1:9999/test.html", ("foo",), 1),
         # request size matches buffer size
-        (None, ("A" * Bootstrapper.BUF_SIZE, timeout), 1),
+        (None, ("A" * Bootstrapper.BUF_SIZE, socket_timeout), 1),
         # large request
         (None, ("A" * Bootstrapper.BUF_SIZE, "foo"), 1),
         # slow startup
-        (None, (timeout, timeout, "foo"), 1),
+        (None, (socket_timeout, socket_timeout, "foo"), 1),
         # slow failed startup with retry
-        (None, (timeout, "", "foo"), 2),
+        (None, (socket_timeout, "", "foo"), 2),
     ],
 )
 def test_bootstrapper_06(mocker, redirect, recv, closed):
@@ -133,8 +132,7 @@ def test_bootstrapper_06(mocker, redirect, recv, closed):
     fake_conn = mocker.Mock(spec_set=socket)
     fake_conn.recv.side_effect = recv
     fake_sock.accept.return_value = (fake_conn, None)
-    mocker.patch("ffpuppet.bootstrapper.socket.socket", return_value=fake_sock)
-    with Bootstrapper() as bts:
+    with Bootstrapper(fake_sock) as bts:
         bts.wait(lambda: True, url=redirect)
     assert fake_conn.close.call_count == closed
     assert fake_conn.recv.call_count == len(recv)
@@ -153,7 +151,7 @@ def _fake_browser(port, payload_size=5120):
             try:
                 conn.connect(("127.0.0.1", port))
                 break
-            except timeout:
+            except socket_timeout:
                 if not attempt:
                     raise
         # send request and receive response
@@ -165,7 +163,7 @@ def _fake_browser(port, payload_size=5120):
         finally:
             conn.close()
 
-    with Bootstrapper() as bts:
+    with Bootstrapper.create() as bts:
         browser_thread = Thread(target=_fake_browser, args=(bts.port,))
         try:
             browser_thread.start()
@@ -184,13 +182,13 @@ def _fake_browser(port, payload_size=5120):
     ],
 )
 def test_bootstrapper_08(mocker, bind, attempts, raised):
-    """test Bootstrapper() - failures"""
+    """test Bootstrapper.create() - failures"""
     mocker.patch("ffpuppet.bootstrapper.sleep", autospec=True)
     fake_sock = mocker.MagicMock(spec_set=socket)
     fake_sock.bind.side_effect = bind
-    mocker.patch("ffpuppet.bootstrapper.socket.socket", return_value=fake_sock)
+    mocker.patch("ffpuppet.bootstrapper.socket", return_value=fake_sock)
     with raises(raised):
-        with Bootstrapper(attempts=attempts):
+        with Bootstrapper.create(attempts=attempts):
             pass
     assert fake_sock.bind.call_count == attempts
     assert fake_sock.close.call_count == attempts
@@ -200,10 +198,10 @@ def test_bootstrapper_09(mocker):
     """test Bootstrapper() - blocked ports"""
     fake_sock = mocker.MagicMock(spec_set=socket)
     fake_sock.getsockname.side_effect = (
-        (None, Bootstrapper.BLOCKED_PORTS[0]),
+        (None, next(iter(Bootstrapper.BLOCKED_PORTS))),
         (None, 12345),
     )
-    mocker.patch("ffpuppet.bootstrapper.socket.socket", return_value=fake_sock)
-    with Bootstrapper(attempts=2):
+    mocker.patch("ffpuppet.bootstrapper.socket", return_value=fake_sock)
+    with Bootstrapper.create(attempts=2):
         pass
     assert fake_sock.close.call_count == 2