[PATCH 2 of 3] killdaemons: use posixfile to avoid intermittent unlink errors on Windows

[Matt Harbison]

On Tue, 16 May 2017 10:29:45 -0400, Yuya Nishihara <yuya at tcha.org> wrote:

> On Mon, 15 May 2017 21:40:24 -0400, Matt Harbison wrote:
>> On Mon, 15 May 2017 10:19:19 -0400, Yuya Nishihara <yuya at tcha.org>  
>> wrote:
>>
>> > On Mon, 15 May 2017 00:08:31 -0400, Matt Harbison wrote:
>> >> # HG changeset patch
>> >> # User Matt Harbison <matt_harbison at yahoo.com>
>> >> # Date 1494808413 14400
>> >> #      Sun May 14 20:33:33 2017 -0400
>> >> # Node ID 809709930080937b6b56e5cad285798f29a10280
>> >> # Parent  024271e90987e5794dab6eb00844467065fae7e4
>> >> killdaemons: use posixfile to avoid intermittent unlink errors on
>> >> Windows
>> >>
>> >> This is the aforementioned fix for the occasional cleanup error with
>> >> #serve
>> >> enabled.  There are a handful of tests that neglect to kill the  
>> daemons
>> >> they
>> >> spawned, and this code is doing a last ditch reap of them.  The test
>> >> that got
>> >> flagged was non-deterministic, and I've seen up to 3 fail in the same
>> >> run.
>> >>
>> >> The problem with trying to import the mercurial module is that while  
>> it
>> >> is
>> >> available for running the individual *.t files, it is not in sys.path
>> >> for
>> >> run-tests.py itself.  I couldn't think of any other way to make this
>> >> work, and
>> >> not affect sys.path for the indiviual tests.  (The main source tree
>> >> _is_ in
>> >> PYTHONPATH when this is imported from run-tests.py.)
>> >>
>> >> diff --git a/tests/killdaemons.py b/tests/killdaemons.py
>> >> --- a/tests/killdaemons.py
>> >> +++ b/tests/killdaemons.py
>> >> @@ -7,6 +7,16 @@
>> >>  import sys
>> >>  import time
>> >>
>> >> +# PYTHONPATH contains the hg source tree when invoked from
>> >> ./run-tests, but
>> >> +# sys.path does not, and importing mercurial fails.  The first  
>> import
>> >> works from
>> >> +# the .t files without editing the path.
>> >> +try:
>> >> +    from mercurial.util import posixfile
>> >> +except ImportError:
>> >> +    srctree =
>> >> os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
>> >> +    sys.path.insert(1, srctree)
>> >> +    from mercurial.util import posixfile
>> >
>> > sys.path is global. I slightly prefer moving this hack to  
>> run-tests.py if
>> > it's okay for run-tests.py to depend on Mercurial modules.
>>
>> I'm not sure if the dependency is OK, but I wasn't sure what else to do.
>> (util.posixfile is a wrapper around osutil.posixfile, so it's not like  
>> it
>> can be copy/pasted.)
>
> Maybe you can copy from the pure version. The whole class is big, but the
> important part would be just a few lines.
>
> If the problem can be mitigated by reading pidfile before killing, we  
> won't
> need a posixfile. But I guess the problem wouldn't be that simple.

I've got to be missing something obvious.  I thought that as long as  
killdaemons() closes the pid file before unlinking it, that would be all  
that is needed.  I'm not sure why a kill in between is a problem.

That said, I changed the loop to populate a list instead of kill()  
directly.  Then close the file, kill each, and finally unlink.  I only had  
time for a couple test runs, but it ran cleanly.