Close the parent process first

This will avoid closing the launcher process on Windows
which can lead to hung processes that do not show up in
task manager. This eventually can lead to a system OOM.

src/ffpuppet/core.py

@@ -271,6 +271,32 @@ def _dbg_sanity_check(cls, dbg: Debugger) -> None:
             if not match or float(match.group("ver")) < cls.VALGRIND_MIN_VERSION:
                 raise OSError(f"Valgrind >= {cls.VALGRIND_MIN_VERSION:.2f} is required")
 
+    @staticmethod
+    def _parent_proc(processes: List[Process]) -> Optional[Process]:
+        """Inspect given processes and select the browser parent process.
+        This assumes all processes are part of the same process tree.
+        This is needed because the process tree is different depending on the platform.
+        Windows:
+            python -> firefox (launcher) -> firefox (parent) -> firefox (content procs)
+
+        Linux and others:
+            python -> firefox (parent) -> firefox (content procs)
+
+        Args:
+            processes: Processes to inspect.
+
+        Returns:
+            The browser parent process if it still exists otherwise false.
+        """
+        for proc in processes:
+            try:
+                cmd = proc.cmdline()
+                if "-contentproc" not in cmd and "-no-deelevate" not in cmd:
+                    return proc
+            except (AccessDenied, NoSuchProcess):  # pragma: no cover
+                pass
+        return None
+
     def _terminate(self) -> None:
         """Call terminate() on browser processes. If terminate() fails try kill().
 
@@ -285,12 +311,12 @@ def _terminate(self) -> None:
             LOG.debug("no processes to terminate")
             return
 
-        assert self._proc
         # try terminating the parent process first, this should be all that is needed
-        if self._proc.poll() is None:
-            LOG.debug("attempting to terminate parent process (%d)", self._proc.pid)
+        parent = self._parent_proc(procs)
+        if parent:
             try:
-                self._proc.terminate()
+                LOG.debug("attempting to terminate parent process (%d)", parent.pid)
+                parent.terminate()
                 # wait for debugger (if in use) and child processes to close
                 procs = wait_procs(procs, timeout=10)[1]
             except (AccessDenied, NoSuchProcess):  # pragma: no cover

src/ffpuppet/test_ffpuppet.py

@@ -836,26 +836,20 @@ def test_ffpuppet_29(mocker):
     # not running
     mocker.patch.object(FFPuppet, "get_processes", return_value=[])
     with FFPuppet() as ffp:
-        ffp._proc = mocker.Mock(spec=Popen)
         ffp._terminate()
-        assert ffp._proc.poll.call_count == 0
-        assert ffp._proc.terminate.call_count == 0
-        ffp._proc = None
 
     proc = mocker.Mock(spec_set=Process, pid=123)
+    proc.cmdline.return_value = [""]
     # running (close with parent)
     mocker.patch.object(FFPuppet, "get_processes", side_effect=([proc],))
     fake_wait_procs.side_effect = (([], []),)
     with FFPuppet() as ffp:
-        ffp._proc = mocker.Mock(spec=Popen, pid=123)
-        ffp._proc.poll.return_value = None
         ffp._terminate()
-        assert ffp._proc.poll.call_count == 1
-        assert ffp._proc.terminate.call_count == 1
-        ffp._proc = None
-    assert proc.terminate.call_count == 0
+    assert proc.cmdline.call_count == 1
+    assert proc.terminate.call_count == 1
     assert fake_wait_procs.call_count == 1
     fake_wait_procs.reset_mock()
+    proc.reset_mock()
 
     # running (terminate() all)
     mocker.patch.object(FFPuppet, "get_processes", side_effect=([proc],))
@@ -864,12 +858,8 @@ def test_ffpuppet_29(mocker):
         ([], []),
     )
     with FFPuppet() as ffp:
-        ffp._proc = mocker.Mock(spec=Popen, pid=123)
-        ffp._proc.poll.return_value = None
         ffp._terminate()
-        assert ffp._proc.poll.call_count == 1
-        ffp._proc = None
-    assert proc.terminate.call_count == 1
+    assert proc.terminate.call_count == 2
     assert proc.kill.call_count == 0
     assert fake_wait_procs.call_count == 2
     fake_wait_procs.reset_mock()
@@ -883,12 +873,8 @@ def test_ffpuppet_29(mocker):
         ([], []),
     )
     with FFPuppet() as ffp:
-        ffp._proc = mocker.Mock(spec=Popen, pid=123)
-        ffp._proc.poll.return_value = None
         ffp._terminate()
-        assert ffp._proc.poll.call_count == 1
-        ffp._proc = None
-    assert proc.terminate.call_count == 1
+    assert proc.terminate.call_count == 2
     assert proc.kill.call_count == 1
     assert fake_wait_procs.call_count == 3
     fake_wait_procs.reset_mock()
@@ -902,12 +888,8 @@ def test_ffpuppet_29(mocker):
         ([], [proc]),
     )
     with FFPuppet() as ffp:
-        ffp._proc = mocker.Mock(spec=Popen, pid=123)
-        ffp._proc.poll.return_value = None
         with raises(TerminateError):
             ffp._terminate()
-        assert ffp._proc.poll.call_count == 1
-        ffp._proc = None
     assert fake_wait_procs.call_count == 3
     fake_wait_procs.reset_mock()
     proc.reset_mock()
@@ -1090,3 +1072,18 @@ def test_ffpuppet_34(mocker):
         proc.environ.return_value = {"FFPUPPET_UID": ffp._uid}
         process_iter.side_effect = ([proc, mocker.Mock(spec_set=Process)],)
         assert any(ffp.get_processes())
+
+
+@mark.parametrize("proc_count", [0, 1, 2])
+def test_ffpuppet_35(mocker, proc_count):
+    """test FFPuppet._parent_proc() setting reason"""
+    procs = []
+    for _ in range(proc_count):
+        procs.append(mocker.Mock(spec_set=Process))
+        procs[-1].cmdline.return_value = [""]
+    parent = FFPuppet._parent_proc(procs)
+    if proc_count:
+        assert parent
+        assert parent.cmdline.call_count == 1
+    else:
+        assert parent is None