Bug 84177 – Lion Production Test failing with error: "Failed to stop httpd: pid file still exists"

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Jer Noble]

run_webkit_tests is failing with the following stack trace:

Traceback (most recent call last):
  File "/Volumes/Data/slave/lion-intel-release-tests/build/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 459, in <module>
    sys.exit(main())
...snip...
    raise http_server_base.ServerError('Failed to stop %s: pid file still exists' % self._name)
webkitpy.layout_tests.servers.http_server_base.ServerError: Failed to stop httpd: pid file still exists
Failed to execute Tools/Scripts/new-run-webkit-tests at ./Tools/Scripts/run-webkit-tests line 122.

[Ryosuke Niwa]

Created attachment 137586 [details]
Try to remove apache's pid file in the case it's stale

[Jer Noble]

Looks like it might be a hardware problem.  Rebooted the offending bot. <http://build.webkit.org/buildslaves/apple-xserve-10>

[Dirk Pranke]

Comment on attachment 137586 [details]
Try to remove apache's pid file in the case it's stale

See the comment right after this ... you can't assume apache has exited until the pid file is removed. If you delete the pid file too early, you may attempt to restart the server while the previous server is still running, and fail. You should only delete the pid file is the pid is gone.

You should probably reuse the log in http_server.py, which checks for the running process.

[Ryosuke Niwa]

Comment on attachment 137586 [details]
Try to remove apache's pid file in the case it's stale

(In reply to comment #3)
> (From update of attachment 137586 [details])
> See the comment right after this ... you can't assume apache has exited until the pid file is removed. If you delete the pid file too early, you may attempt to restart the server while the previous server is still running, and fail. You should only delete the pid file is the pid is gone.
> 
> You should probably reuse the log in http_server.py, which checks for the running process.

Okay, please make those changes then. I don't understand this code and don't have a time for it.

[Jer Noble]

Created attachment 137753 [details]
Patch

[Dirk Pranke]

Comment on attachment 137753 [details]
Patch

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.

[Jer Noble]

Created attachment 137757 [details]
Patch

Updated with dpranke's rewrite.

[Dirk Pranke]

Comment on attachment 137757 [details]
Patch

r+ if you add a "return" after line 143 so that we don't execute the rest of the function.

[Ryosuke Niwa]

Thanks for the fix!

[Jer Noble]

Committed r114555: <http://trac.webkit.org/changeset/114555>