Bug 1493796 - [mozprocess] Revert poll() behavior on Windows due to regression. r=gbrown
authorHenrik Skupin <mail@hskupin.info>
Thu, 18 Oct 2018 15:32:28 +0000
changeset 497731 9782e0b1657da7b2742e693222f8ac912920b521
parent 497730 740c3623a44aed64203d830a03682e14f90917f9
child 497732 b5b389d5788ed89f7e666c4de6e80e6a3cb99a19
push id10002
push userarchaeopteryx@coole-files.de
push dateFri, 19 Oct 2018 23:09:29 +0000
treeherdermozilla-beta@01378c910610 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgbrown
bugs1493796, 1433905
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1493796 - [mozprocess] Revert poll() behavior on Windows due to regression. r=gbrown Originally landed as changeset 8793e332890e via bug 1433905 the patch caused a regression because GetExitCodeProcess() returns 0 for an inside of Firefox restarted process. It can be relanded once the process id of the job object can successfully be tracked. Differential Revision: https://phabricator.services.mozilla.com/D9020
testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
testing/mozbase/mozprocess/mozprocess/processhandler.py
testing/mozbase/mozprocess/tests/test_poll.py
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
@@ -1,14 +1,16 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # 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/.
 
 from __future__ import absolute_import, print_function
 
+import sys
+import unittest
 import urllib
 
 from unittest import skip
 
 from marionette_driver import errors
 from marionette_driver.by import By
 from marionette_harness import MarionetteTestCase
 
@@ -191,27 +193,29 @@ class TestQuitRestart(MarionetteTestCase
 
         self.assertNotEqual(self.marionette.get_pref("startup.homepage_welcome_url"),
                             "about:about")
 
     def test_in_app_restart_with_callback_not_callable(self):
         with self.assertRaisesRegexp(ValueError, "is not callable"):
             self.marionette.restart(in_app=True, callback=4)
 
+    @unittest.skipIf(sys.platform.startswith("win"), "Bug 1493796")
     def test_in_app_restart_with_callback_but_process_quit(self):
         timeout_shutdown = self.marionette.DEFAULT_SHUTDOWN_TIMEOUT
         # Wait at least 70s for the hang monitor in case of a shutdown hang
         self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = 70
 
         try:
             with self.assertRaisesRegexp(IOError, "Process unexpectedly quit without restarting"):
                 self.marionette.restart(in_app=True, callback=lambda: self.shutdown(restart=False))
         finally:
             self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = timeout_shutdown
 
+    @unittest.skipIf(sys.platform.startswith("win"), "Bug 1493796")
     def test_in_app_restart_with_callback_missing_shutdown(self):
         try:
             timeout_shutdown = self.marionette.DEFAULT_SHUTDOWN_TIMEOUT
             self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = 5
 
             with self.assertRaisesRegexp(IOError, "the connection to Marionette server is lost"):
                 self.marionette.restart(in_app=True, callback=lambda: False)
         finally:
--- a/testing/mozbase/mozprocess/mozprocess/processhandler.py
+++ b/testing/mozbase/mozprocess/mozprocess/processhandler.py
@@ -200,37 +200,24 @@ class ProcessHandlerMixin(object):
                     # a signal was explicitly set or not posix
                     send_sig(sig or signal.SIGKILL)
 
             self.returncode = self.wait()
             self._cleanup()
             return self.returncode
 
         def poll(self):
-            """Check if child process has terminated, and set returncode attribute.
-
-            This method overrides the Popen.poll implementation for our custom
-            process handling for Windows.
+            """ Popen.poll
+                Check if child process has terminated. Set and return returncode attribute.
             """
-            if isWin:
-                # If we have a handle, check that the process is still alive.
-                if self._handle:
-                    returncode = winprocess.GetExitCodeProcess(self._handle)
+            # If we have a handle, the process is alive
+            if isWin and getattr(self, '_handle', None):
+                return None
 
-                    # If the process doesn't exist anymore run cleanup steps
-                    if returncode != winprocess.STILL_ACTIVE:
-                        self.returncode = returncode
-                        self._cleanup()
-                    else:
-                        self.returncode = None
-
-            else:
-                self.returncode = subprocess.Popen.poll(self)
-
-            return self.returncode
+            return subprocess.Popen.poll(self)
 
         def wait(self):
             """ Popen.wait
                 Called to wait for a running process to shut down and return
                 its exit code
                 Returns the main process's exit code
             """
             # This call will be different for each OS
--- a/testing/mozbase/mozprocess/tests/test_poll.py
+++ b/testing/mozbase/mozprocess/tests/test_poll.py
@@ -1,15 +1,17 @@
 #!/usr/bin/env python
 
 from __future__ import absolute_import
 
 import os
 import signal
+import sys
 import time
+import unittest
 
 import mozinfo
 import mozunit
 
 from mozprocess import processhandler
 
 import proctest
 
@@ -98,16 +100,17 @@ class ProcTestPoll(proctest.ProcTest):
         else:
             self.assertLess(returncode, 0,
                             'Negative returncode expected, got "%s"' % returncode)
 
         self.assertEqual(returncode, p.poll())
 
         self.determine_status(p)
 
+    @unittest.skipIf(sys.platform.startswith("win"), "Bug 1493796")
     def test_poll_after_external_kill(self):
         """Process is killed externally, and poll() is called."""
         p = processhandler.ProcessHandler([self.python, self.proclaunch,
                                            "process_normal_finish.ini"],
                                           cwd=here)
         p.run()
 
         os.kill(p.pid, signal.SIGTERM)