Bug 1571426 - Read geckodriver port on startup for all wdspec tests, r=webdriver-reviewers,jdescottes

Instead of using get_free_port(), pass in 0 as the port number and
allow geckodriver to select a free port. Then read the port from the
geckodriver output at startup. This should prevent rare race
conditions where we happen to select a port that later turns out to be
used.

Differential Revision: https://phabricator.services.mozilla.com/D189927

Author: jgraham

testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/base.py

@@ -5,6 +5,7 @@
 import os
 import platform
 import socket
+import time
 import traceback
 from abc import ABCMeta, abstractmethod
 
@@ -302,8 +303,7 @@ def __init__(self, logger, binary=None, webdriver_binary=None,
         self.env = os.environ.copy() if env is None else env
         self.webdriver_args = webdriver_args if webdriver_args is not None else []
 
-        self.url = f"http://{self.host}:{self.port}{self.base_path}"
-
+        self.init_deadline = None
         self._output_handler = None
         self._cmd = None
         self._proc = None
@@ -314,10 +314,12 @@ def make_command(self):
         return [self.webdriver_binary] + self.webdriver_args
 
     def start(self, group_metadata, **kwargs):
+        self.init_deadline = time.time() + self.init_timeout
         try:
             self._run_server(group_metadata, **kwargs)
         except KeyboardInterrupt:
             self.stop()
+            raise
 
     def create_output_handler(self, cmd):
         """Return an instance of the class used to handle application output.
@@ -351,7 +353,7 @@ def _run_server(self, group_metadata, **kwargs):
                 self.logger,
                 self.host,
                 self.port,
-                timeout=self.init_timeout,
+                timeout=self.init_deadline - time.time(),
                 server_process=self._proc,
             )
         except Exception:
@@ -384,6 +386,12 @@ def stop(self, force=False):
     def is_alive(self):
         return hasattr(self._proc, "proc") and self._proc.poll() is None
 
+    @property
+    def url(self):
+        if self.port is not None:
+            return f"http://{self.host}:{self.port}{self.base_path}"
+        raise ValueError("Can't get WebDriver URL before port is assigned")
+
     @property
     def pid(self):
         if self._proc is not None:

testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/firefox.py

@@ -3,6 +3,7 @@
 import json
 import os
 import platform
+import re
 import signal
 import subprocess
 import sys
@@ -604,6 +605,32 @@ def __call__(self, line):
                                            command=" ".join(self.command))
 
 
+class GeckodriverOutputHandler(FirefoxOutputHandler):
+    PORT_RE = re.compile(b".*Listening on [^ :]*:(\d+)")
+
+    def __init__(self, logger, command, symbols_path=None, stackfix_dir=None, asan=False,
+                 leak_report_file=None, init_deadline=None):
+        super().__init__(logger, command, symbols_path=symbols_path, stackfix_dir=stackfix_dir, asan=asan,
+                         leak_report_file=leak_report_file)
+        self.port = None
+        self.init_deadline = None
+
+    def after_process_start(self, pid):
+        super().after_process_start(pid)
+        while self.port is None:
+            time.sleep(0.1)
+            if self.init_deadline is not None and time.time() > self.init_deadline:
+                raise TimeoutError("Failed to get geckodriver port within the timeout")
+
+    def __call__(self, line):
+        if self.port is None:
+            m = self.PORT_RE.match(line)
+            if m is not None:
+                self.port = int(m.groups()[0])
+                self.logger.debug(f"Got geckodriver port {self.port}")
+        super().__call__(line)
+
+
 class ProfileCreator:
     def __init__(self, logger, prefs_root, config, test_type, extra_prefs, e10s,
                  disable_fission, debug_test, browser_channel, binary, certutil_binary,
@@ -922,12 +949,13 @@ def get_env(self, binary, debug_info, headless, chaos_mode_flags):
         return env
 
     def create_output_handler(self, cmd):
-        return FirefoxOutputHandler(self.logger,
-                                    cmd,
-                                    stackfix_dir=self.stackfix_dir,
-                                    symbols_path=self.symbols_path,
-                                    asan=self.asan,
-                                    leak_report_file=self.leak_report_file)
+        return GeckodriverOutputHandler(self.logger,
+                                        cmd,
+                                        stackfix_dir=self.stackfix_dir,
+                                        symbols_path=self.symbols_path,
+                                        asan=self.asan,
+                                        leak_report_file=self.leak_report_file,
+                                        init_deadline=self.init_deadline)
 
     def start(self, group_metadata, **kwargs):
         self.leak_report_file = setup_leak_report(self.leak_check, self.profile, self.env)
@@ -971,7 +999,12 @@ def stop(self, force=False):
                 time.sleep(1)
             else:
                 self.logger.debug("WebDriver session didn't end")
-        super().stop(force=force)
+        try:
+            super().stop(force=force)
+        finally:
+            if self._output_handler is not None:
+                self._output_handler.port = None
+            self._port = None
 
     def cleanup(self):
         super().cleanup()
@@ -985,10 +1018,19 @@ def settings(self, test):
                 "mozleak_allowed": self.leak_check and test.mozleak_allowed,
                 "mozleak_thresholds": self.leak_check and test.mozleak_threshold}
 
+    @property
+    def port(self):
+        # We read the port from geckodriver on startup
+        if self._port is None:
+            if self._output_handler is None or self._output_handler.port is None:
+                raise ValueError("Can't get geckodriver port before it's started")
+            self._port = self._output_handler.port
+        return self._port
+
     def make_command(self):
         return [self.webdriver_binary,
                 "--host", self.host,
-                "--port", str(self.port)] + self.webdriver_args
+                "--port", "0"] + self.webdriver_args
 
     def executor_browser(self):
         cls, args = super().executor_browser()