[webkit-reviews] review denied: [Bug 84177] Lion Production Test failing with error: "Failed to stop httpd: pid file still exists" : [Attachment 137753] Patch

[bugzilla-daemon at webkit.org]

Dirk Pranke <dpranke at chromium.org> has denied Jer Noble <jer.noble at apple.com>'s
request for review:
Bug 84177: Lion Production Test failing with error: "Failed to stop httpd: pid
file still exists"
https://bugs.webkit.org/show_bug.cgi?id=84177

Attachment 137753: Patch
https://bugs.webkit.org/attachment.cgi?id=137753&action=review

------- Additional Comments from Dirk Pranke <dpranke at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=137753&action=review


> Tools/Scripts/webkitpy/layout_tests/servers/apache_http_server.py:144
> +	   # If apache was forcefully killed, the pid file will not have been
deleted, so check

At the beginning of this routine, (before we call self._run(self._stop_cmd), we
should check to see if the server is running ... in the context of the problem
we're seeing, we know that self._pid is set to the pid we care about (see
start() in http_server_base.py:78).

So, I would rewrite the whole routine as something like:

def _stop_running_server(self):
    if self._pid and not self._executive.check_running_pid(pid):
	self._filesystem.remove(self._pid_file)
	return

   retval, err = self._run(self.stop_cmd)
   if retval or len(err):
       raise http_server_base.ServerError('Failed to stop %s: %s' %
(self._name, err))
  # For some reason ...
  ...

In this case you'll raise an error if you can't delete the pidfile, but I think
that's probably good.