Http lock doesn't work on Windows platform.
Created attachment 71875 [details] proposed patch
Comment on attachment 71875 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=71875&action=review LGTM with the comment fixed as per below. > WebKitTools/Scripts/webkitpy/layout_tests/port/http_lock.py:75 > + FIXME: On Windows it's always return True.""" Nit: I'd rewrite this as "FIXME: os.kill() doesn't work on Windows for checking if a pid is alive, so always return True"
Comment on attachment 71875 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=71875&action=review > WebKitTools/Scripts/webkitpy/layout_tests/port/http_lock.py:78 > + FIXME: On Windows it's always return True.""" > + if sys.platform in ('darwin', 'linux2'): > + try: > + os.kill(current_pid, 0) In webkitpy/common/system/executive.py, there's a kill_process function that should work on windows. Can we use that instead? > WebKitTools/Scripts/webkitpy/layout_tests/port/http_lock.py:108 > + if sys.platform in ('darwin', 'linux2'): > + guard_lock_flags |= os.O_NONBLOCK Should we add 'cygwin' to the platform tuple? It looks like cygwin python has os.O_NONBLOCK, although I'm not sure if it does anything.
(In reply to comment #3) > (From update of attachment 71875 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=71875&action=review > > > WebKitTools/Scripts/webkitpy/layout_tests/port/http_lock.py:78 > > + FIXME: On Windows it's always return True.""" > > + if sys.platform in ('darwin', 'linux2'): > > + try: > > + os.kill(current_pid, 0) > > In webkitpy/common/system/executive.py, there's a kill_process function that should work on windows. Can we use that instead? > No. That routine actually kills the process. This routine is just probing the process to see if it's alive. Frankly, I'm not a fan of using kill -0 to do that, but there doesn't seem to be an alternative that doesn't require shelling out to ps or invoking WMI on windows. > > WebKitTools/Scripts/webkitpy/layout_tests/port/http_lock.py:108 > > + if sys.platform in ('darwin', 'linux2'): > > + guard_lock_flags |= os.O_NONBLOCK > > Should we add 'cygwin' to the platform tuple? It looks like cygwin python has os.O_NONBLOCK, although I'm not sure if it does anything. I doubt it does anything.
We started the process. So we should have a Popen object no? If so, we can just use poll() to check if a process is alive.
(In reply to comment #5) > We started the process. So we should have a Popen object no? If so, we can just use poll() to check if a process is alive. Not if you're running multiple NRWTs and one of the other ones is the one holding the lock ...
Created attachment 72003 [details] proposed_patch_v2 As Dirk said I don't want to kill the process, just check it. I think that's the easiest way to do that. Furthermore I've checked the os.O_NONBLOCK and it's no need because when it can't open the guard lock file it just throw an OSError exception, so it will be a spinlock without the os.O_NONBLOCK.
Comment on attachment 72003 [details] proposed_patch_v2 Clearing flags on attachment: 72003 Committed r70672: <http://trac.webkit.org/changeset/70672>
All reviewed patches have been landed. Closing bug.