Fixes (#1084)

* refactor(FirefoxService): remove patched firefox service

* fix(deploy_firefox.py): remove race condition in log interceptor

openwpm/deploy_browsers/deploy_firefox.py

@@ -125,8 +125,15 @@ def deploy_firefox(
 
     # Intercept logging at the Selenium level and redirect it to the
     # main logger.
-    interceptor = FirefoxLogInterceptor(browser_params.browser_id)
-    interceptor.start()
+    webdriver_interceptor = FirefoxLogInterceptor(
+        browser_params.browser_id, is_webdriver=True
+    )
+    webdriver_interceptor.start()
+
+    browser_interceptor = FirefoxLogInterceptor(
+        browser_params.browser_id, is_webdriver=False
+    )
+    browser_interceptor.start()
 
     # Set custom prefs. These are set after all of the default prefs to allow
     # our defaults to be overwritten.
@@ -141,15 +148,16 @@ def deploy_firefox(
     status_queue.put(("STATUS", "Launch Attempted", None))
 
     fo.binary = FirefoxBinary(
-        firefox_path=firefox_binary_path, log_file=open(interceptor.fifo, "w")
+        firefox_path=firefox_binary_path, log_file=open(browser_interceptor.fifo, "w")
     )
     geckodriver_path = subprocess.check_output(
         "which geckodriver", encoding="utf-8", shell=True
     ).strip()
     driver = webdriver.Firefox(
         options=fo,
         service=Service(
-            executable_path=geckodriver_path, log_output=open(interceptor.fifo, "w")
+            executable_path=geckodriver_path,
+            log_output=open(webdriver_interceptor.fifo, "w"),
         ),
     )
 

openwpm/deploy_browsers/selenium_firefox.py

@@ -9,7 +9,6 @@
 import tempfile
 import threading
 
-from selenium.webdriver.firefox import service as FirefoxServiceModule
 from selenium.webdriver.firefox.firefox_binary import FirefoxBinary
 from selenium.webdriver.firefox.options import Options
 
@@ -50,12 +49,16 @@ class FirefoxLogInterceptor(threading.Thread):
     instance.
     """
 
-    def __init__(self, browser_id: BrowserId) -> None:
-        threading.Thread.__init__(self, name="log-interceptor-%i" % browser_id)
+    def __init__(self, browser_id: BrowserId, is_webdriver: bool) -> None:
+        threading.Thread.__init__(
+            self,
+            name=f"log-interceptor-{'webdriver' if is_webdriver else 'browser'}-{browser_id}",
+        )
         self.browser_id = browser_id
         self.fifo = mktempfifo(suffix=".log", prefix="owpm_driver_")
         self.daemon = True
         self.logger = logging.getLogger("openwpm")
+        assert self.fifo is not None
 
     def run(self) -> None:
         # We might not ever get EOF on the FIFO, so instead we delete
@@ -71,38 +74,9 @@ def run(self) -> None:
                     if self.fifo is not None:
                         os.unlink(self.fifo)
                         self.fifo = None
-
+        except Exception:
+            self.logger.error("Error in LogInterceptor", exc_info=True)
         finally:
             if self.fifo is not None:
                 os.unlink(self.fifo)
                 self.fifo = None
-
-
-class PatchedFirefoxService(FirefoxServiceModule.Service):
-    """Object that manages the starting and stopping of the GeckoDriver.
-    We need to override the constructor to be able to write to the FIFO
-    queue we use for log collection
-    """
-
-    def __init__(
-        self,
-        executable_path,
-        port=0,
-        service_args=None,
-        log_path="geckodriver.log",
-        env=None,
-    ):
-        super().__init__(executable_path, port, service_args, log_path, env)
-        if self.log_file:
-            os.close(self.log_file)
-
-        if log_path:
-            try:
-                self.log_file = open(log_path, "a")
-            except OSError as e:
-                if e.errno != errno.ESPIPE:
-                    raise
-                self.log_file = open(log_path, "w")
-
-
-FirefoxServiceModule.Service = PatchedFirefoxService