Bug 84177

Summary: Lion Production Test failing with error: "Failed to stop httpd: pid file still exists"
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: Tools / TestsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dpranke, ojan, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://build.webkit.org/builders/Lion%20Release%20%28Tests%29/builds/7667/steps/layout-test/logs/stdio
Attachments:
Description Flags
Try to remove apache's pid file in the case it's stale
dpranke: review-
Patch
none
Patch dpranke: review+

Jer Noble
Reported 2012-04-17 11:55:49 PDT
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.
Attachments
Try to remove apache's pid file in the case it's stale (1.72 KB, patch)
2012-04-17 12:59 PDT, Ryosuke Niwa
dpranke: review-
Patch (2.04 KB, patch)
2012-04-18 13:17 PDT, Jer Noble
no flags
Patch (1.82 KB, patch)
2012-04-18 13:43 PDT, Jer Noble
dpranke: review+
Ryosuke Niwa
Comment 1 2012-04-17 12:59:31 PDT
Created attachment 137586 [details] Try to remove apache's pid file in the case it's stale
Jer Noble
Comment 2 2012-04-17 13:03:02 PDT
Looks like it might be a hardware problem. Rebooted the offending bot. <http://build.webkit.org/buildslaves/apple-xserve-10>
Dirk Pranke
Comment 3 2012-04-17 13:20:44 PDT
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 4 2012-04-17 13:22:15 PDT
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
Comment 5 2012-04-18 13:17:24 PDT
Dirk Pranke
Comment 6 2012-04-18 13:34:52 PDT
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
Comment 7 2012-04-18 13:43:06 PDT
Created attachment 137757 [details] Patch Updated with dpranke's rewrite.
Dirk Pranke
Comment 8 2012-04-18 13:44:43 PDT
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
Comment 9 2012-04-18 14:47:23 PDT
Thanks for the fix!
Jer Noble
Comment 10 2012-04-20 12:34:33 PDT
Note You need to log in before you can comment on or make changes to this bug.