Changeset 63476 in webkit

Timestamp:

Jul 15, 2010 3:29:44 PM (14 years ago)

Author:

Adam Roben

Message:

Make killing Apache more reliable (on both Mac and Windows)

We previously had two ways of determining whether we had succeeded in
killing Apache:

1) checking the return value of kill(0, $apachePID)
2) checking whether Apache's PID file still exists

On Cygwin, Apache doesn't always delete its PID file when it exits,
making (2) unreliable. We unfortunately misdiagnosed this as an
impotency of Perl's kill function, which led to r63177 and r63355.

Now that we know that the real problem is that Apache doesn't always
delete its PID file on Windows, we can make a much better fix: always
use method (1) to determine whether we've killed Apache.

Fixes <​http://webkit.org/b/42415> Killing Apache is unreliable,
leading to regression test failures (and general annoyance).

Reviewed by Anders Carlsson.

• Scripts/webkitperl/httpd.pm:

(openHTTPD): Moved killing code from here to killHTTPD. Added a call
to delete the PID file in case Apache doesn't do this itself when
killed. Our later logic relies on the PID file being deleted after
this point.
(closeHTTPD): Removed killing logic and changed to just call killHTTPD
instead. killHTTPD's logic is a bit different from the logic we had
here, for the reasons stated above.
(killHTTPD): Added. Code came from openHTTPD.
(handleInterrupt): Bonus fix for Mac: don't hang when pressing Ctrl-C!
On Mac, don't try to kill Apache when we receive a signal, as Apache
will already have been killed by this point (though for some reason
this isn't detected by our killing logic in killHTTPD). On Cygwin, we
still need to kill Apache manually.

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitperl/httpd.pm (modified) (3 diffs)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2010-07-15  Adam Roben  <aroben@apple.com>
+
+        Make killing Apache more reliable (on both Mac and Windows)
+
+        We previously had two ways of determining whether we had succeeded in
+        killing Apache:
+          1) checking the return value of kill(0, $apachePID)
+          2) checking whether Apache's PID file still exists
+
+        On Cygwin, Apache doesn't always delete its PID file when it exits,
+        making (2) unreliable. We unfortunately misdiagnosed this as an
+        impotency of Perl's kill function, which led to r63177 and r63355.
+
+        Now that we know that the real problem is that Apache doesn't always
+        delete its PID file on Windows, we can make a much better fix: always
+        use method (1) to determine whether we've killed Apache.
+
+        Fixes <http://webkit.org/b/42415> Killing Apache is unreliable,
+        leading to regression test failures (and general annoyance).
+
+        Reviewed by Anders Carlsson.
+
+        * Scripts/webkitperl/httpd.pm:
+        (openHTTPD): Moved killing code from here to killHTTPD. Added a call
+        to delete the PID file in case Apache doesn't do this itself when
+        killed. Our later logic relies on the PID file being deleted after
+        this point.
+        (closeHTTPD): Removed killing logic and changed to just call killHTTPD
+        instead. killHTTPD's logic is a bit different from the logic we had
+        here, for the reasons stated above.
+        (killHTTPD): Added. Code came from openHTTPD.
+        (handleInterrupt): Bonus fix for Mac: don't hang when pressing Ctrl-C!
+        On Mac, don't try to kill Apache when we receive a signal, as Apache
+        will already have been killed by this point (though for some reason
+        this isn't detected by our killing logic in killHTTPD). On Cygwin, we
+        still need to kill Apache manually.
+
 2010-07-15  Sam Weinig  <sam@webkit.org>
 

trunk/WebKitTools/Scripts/webkitperl/httpd.pm

@@ -155 +155 @@
         if (0 != kill 0, $oldPid) {
             print "\nhttpd is already running: pid $oldPid, killing...\n";
-            kill 15, $oldPid;
-
-            my $retryCount = 20;
-            while ((kill(0, $oldPid) != 0) && $retryCount) {
-                sleep 1;
-                --$retryCount;
-            }
-
-            if (!$retryCount) {
+            if (!killHTTPD($oldPid)) {
                 cleanUp();
                 die "Timed out waiting for httpd to quit";
             }
         }
+        unlink $httpdPidFile;
     }
 
@@ -197 +190 @@
 {
     close HTTPDIN;
+    my $succeeded = killHTTPD($httpdPid);
+    cleanUp();
+    unless ($succeeded) {
+        print STDERR "Timed out waiting for httpd to terminate!\n" unless $succeeded;
+        return 0;
+    }
+    return 1;
+}
+
+sub killHTTPD
+{
+    my ($pid) = @_;
+
+    return 1 unless $pid;
+
+    kill 15, $pid;
+
     my $retryCount = 20;
-    if ($httpdPid) {
-        if (isCygwin()) {
-            # Kill the process (and all its child processes) using taskkill, as
-            # perl's kill doesn't seem to work with Apache. A return value of 0
-            # means the process was killed, and return value of 32768 means
-            # there was no process with that PID. We use open/close here
-            # instead of system to avoid having taskkill print to stdout.
-            if (open(TASKKILL, "-|", qw(taskkill /f /t /pid), $httpdPid) && close(TASKKILL) && (!$? || $? == 32768)) {
-                # Cygwin's Apache doesn't delete the PID file itself when
-                # killed. We delete the PID file for it to work around this.
-                unlink $httpdPidFile;
-            }
-        } else {
-            kill 15, $httpdPid;
-        }
-        while (-f $httpdPidFile && $retryCount) {
-            sleep 1;
-            --$retryCount;
-        }
-    }
-    cleanUp();
-    if (!$retryCount) {
-        print STDERR "Timed out waiting for httpd to terminate!\n";
-        return 0;
-    }
-    return 1;
+    while (kill(0, $pid) && $retryCount) {
+        sleep 1;
+        --$retryCount;
+    }
+    return $retryCount != 0;
 }
 
@@ -233 +222 @@
 sub handleInterrupt
 {
-    closeHTTPD();
+    # On Cygwin, when we receive a signal Apache is still running, so we need
+    # to kill it. On other platforms (at least Mac OS X), Apache will have
+    # already been killed, and trying to kill it again will cause us to hang.
+    # All we need to do in this case is clean up our own files.
+    if (isCygwin()) {
+        closeHTTPD();
+    } else {
+        cleanUp();
+    }
+
     print "\n";
     exit(1);