[py] fix shutdown and process termination - #3263
Conversation
|
/cc @AutomatedTester |
| def __del__(self): | ||
| # subprocess.Popen doesn't send signal on __del__; | ||
| # we have to try to stop the launched process. | ||
| self.stop() |
There was a problem hiding this comment.
can we add a try / except around this to make it more stable?
I like the idea of any mistakenly un-quit drivers closing down after the process is quit (the java server does this... also i forget to do driver.quit often when i use the command line repl :) )
There was a problem hiding this comment.
yea, i think that just swallowing exceptions from a try/except is ok here.
I've updated my branch and re-added the del
p.s. I'll get that dastardly __del__ out of there eventually.... fear not! :)
p.p.s. sorry I also piggybacked the tiny style change to the connection test in send_remote_shutdown_command... it was so egregiously un-pythonic. :)
|
oops.. this should be: |
|
@lukeis can you fix? |
|
and this is why i tried to stay away from python :-D |
|
arg.. im all over the place :( it should be: |
michaelfward
left a comment
There was a problem hiding this comment.
Thank you so much for finding this. I've been looking for months. Great work!
This PR addresses 2 issues with how webdriver stops services from the python bindings.
Changes the order of events to stop a process. Previously. the order was
terminate()->kill()->wait(). So... it would send a SIGTERM signal to the process, immediately followed by a SIGKILL signal. Sincewait()is called after the process has already been forcefully stopped, there is nothing to wait on, so it does nothing. The proper sequence should be:terminate()->wait()->kill(). That will send a SIGINT, wait for graceful process termination, then send a SIGKILL to forcefully stop it as a last resort.Removes the__del__()method from theService()class. The existence of this method is causing _cookie_temp_file being closed and removed twice #3216. Using__del__()to callstop()is unreliable and not a good idea in Python.The proper way to stop the service is to explicitly call the
stop()methodAdd a try/except inside
__del__()so inconsequential exceptions arent displayedXin the preceding checkbox, I verify that I have signed the Contributor License Agreement