[nrwt] handle corrupt http server pid files cleanly
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Mar 2013 20:58:57 +0000 (20:58 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Mar 2013 20:58:57 +0000 (20:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=111628

Reviewed by Eric Seidel.

If the pid file from a previous http server is corrupt for some
reason, NRWT will just raise errors and not clean it up or recover.
This patch fixes that to at least delete the pid file and
not throw; not that we may still have stale http servers left on
the system, since there's no way to know which pid to kill if
the file was corrupted.

* Scripts/webkitpy/layout_tests/servers/http_server_base.py:
(HttpServerBase.start):
(HttpServerBase.stop):
(HttpServerBase._remove_pid_file):
* Scripts/webkitpy/layout_tests/servers/http_server_base_unittest.py: Added.
(TestHttpServerBase):
(TestHttpServerBase.test_corrupt_pid_file):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@145122 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/layout_tests/servers/http_server_base.py
Tools/Scripts/webkitpy/layout_tests/servers/http_server_base_unittest.py [new file with mode: 0644]

index 76ca515..1a0b235 100644 (file)
@@ -1,3 +1,25 @@
+2013-03-07  Dirk Pranke  <dpranke@chromium.org>
+
+        [nrwt] handle corrupt http server pid files cleanly
+        https://bugs.webkit.org/show_bug.cgi?id=111628
+
+        Reviewed by Eric Seidel.
+
+        If the pid file from a previous http server is corrupt for some
+        reason, NRWT will just raise errors and not clean it up or recover.
+        This patch fixes that to at least delete the pid file and
+        not throw; not that we may still have stale http servers left on
+        the system, since there's no way to know which pid to kill if
+        the file was corrupted.
+
+        * Scripts/webkitpy/layout_tests/servers/http_server_base.py:
+        (HttpServerBase.start):
+        (HttpServerBase.stop):
+        (HttpServerBase._remove_pid_file):
+        * Scripts/webkitpy/layout_tests/servers/http_server_base_unittest.py: Added.
+        (TestHttpServerBase):
+        (TestHttpServerBase.test_corrupt_pid_file):
+
 2013-03-07  Chris Fleizach  <cfleizach@apple.com>
 
         AX: Can't activate links with VoiceOver in Safari
index 432c72e..3ce15a5 100644 (file)
@@ -75,8 +75,12 @@ class HttpServerBase(object):
 
         # Stop any stale servers left over from previous instances.
         if self._filesystem.exists(self._pid_file):
-            self._pid = int(self._filesystem.read_text_file(self._pid_file))
-            self._stop_running_server()
+            try:
+                self._pid = int(self._filesystem.read_text_file(self._pid_file))
+                self._stop_running_server()
+            except (ValueError, UnicodeDecodeError):
+                # These could be raised if the pid file is corrupt.
+                self._remove_pid_file()
             self._pid = None
 
         self._remove_stale_logs()
@@ -94,29 +98,37 @@ class HttpServerBase(object):
     def stop(self):
         """Stops the server. Stopping a server that isn't started is harmless."""
         actual_pid = None
-        if self._filesystem.exists(self._pid_file):
-            actual_pid = int(self._filesystem.read_text_file(self._pid_file))
+        try:
+            if self._filesystem.exists(self._pid_file):
+                try:
+                    actual_pid = int(self._filesystem.read_text_file(self._pid_file))
+                except (ValueError, UnicodeDecodeError):
+                    # These could be raised if the pid file is corrupt.
+                    pass
+                if not self._pid:
+                    self._pid = actual_pid
+
             if not self._pid:
-                self._pid = actual_pid
-
-        if not self._pid:
-            return
-
-        if not actual_pid:
-            _log.warning('Failed to stop %s: pid file is missing' % self._name)
-            return
-        if self._pid != actual_pid:
-            _log.warning('Failed to stop %s: pid file contains %d, not %d' %
-                         (self._name, actual_pid, self._pid))
-            # Try to kill the existing pid, anyway, in case it got orphaned.
-            self._executive.kill_process(self._pid)
+                return
+
+            if not actual_pid:
+                _log.warning('Failed to stop %s: pid file is missing' % self._name)
+                return
+            if self._pid != actual_pid:
+                _log.warning('Failed to stop %s: pid file contains %d, not %d' %
+                            (self._name, actual_pid, self._pid))
+                # Try to kill the existing pid, anyway, in case it got orphaned.
+                self._executive.kill_process(self._pid)
+                self._pid = None
+                return
+
+            _log.debug("Attempting to shut down %s server at pid %d" % (self._name, self._pid))
+            self._stop_running_server()
+            _log.debug("%s server at pid %d stopped" % (self._name, self._pid))
             self._pid = None
-            return
-
-        _log.debug("Attempting to shut down %s server at pid %d" % (self._name, self._pid))
-        self._stop_running_server()
-        _log.debug("%s server at pid %d stopped" % (self._name, self._pid))
-        self._pid = None
+        finally:
+            # Make sure we delete the pid file no matter what happens.
+            self._remove_pid_file()
 
     def _prepare_config(self):
         """This routine can be overridden by subclasses to do any sort
@@ -143,6 +155,10 @@ class HttpServerBase(object):
 
     # Utility routines.
 
+    def _remove_pid_file(self):
+        if self._filesystem.exists(self._pid_file):
+            self._filesystem.remove(self._pid_file)
+
     def _remove_log_files(self, folder, starts_with):
         files = self._filesystem.listdir(folder)
         for file in files:
diff --git a/Tools/Scripts/webkitpy/layout_tests/servers/http_server_base_unittest.py b/Tools/Scripts/webkitpy/layout_tests/servers/http_server_base_unittest.py
new file mode 100644 (file)
index 0000000..410f6e4
--- /dev/null
@@ -0,0 +1,58 @@
+# Copyright (C) 2012 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#     * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#     * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+import unittest2 as unittest
+
+from webkitpy.common.host_mock import MockHost
+from webkitpy.layout_tests.port import test
+from webkitpy.layout_tests.servers.http_server_base import HttpServerBase
+
+
+class TestHttpServerBase(unittest.TestCase):
+    def test_corrupt_pid_file(self):
+        # This tests that if the pid file is corrupt or invalid,
+        # both start() and stop() deal with it correctly and delete the file.
+        host = MockHost()
+        test_port = test.TestPort(host)
+
+        server = HttpServerBase(test_port)
+        server._pid_file = '/tmp/pidfile'
+        server._spawn_process = lambda: 4
+        server._is_server_running_on_all_ports = lambda: True
+
+        host.filesystem.write_text_file(server._pid_file, 'foo')
+        server.stop()
+        self.assertEqual(host.filesystem.files[server._pid_file], None)
+
+        host.filesystem.write_text_file(server._pid_file, 'foo')
+        server.start()
+        self.assertEqual(server._pid, 4)
+
+        # Note that the pid file would not be None if _spawn_process()
+        # was actually a real implementation.
+        self.assertEqual(host.filesystem.files[server._pid_file], None)