bug#10470: MSYS: race in directory access?

[Peter Rosin]

Stefano Lattarini skrev 2012-01-10 10:50:
> Hi Peter.
> 
> On 01/10/2012 12:15 AM, Peter Rosin wrote:
>> Peter Rosin skrev 2012-01-10 00:07:
>>> Hi!
>>>
>>> distcheck-configure-flags-am.test fails on MSYS.
>>
> Is this failure consistently reproducible, or racy?  If racy, do you have
> an estimate of its frequency?

If the analysis is correct and it really is some other process that "sits"
in the to be removed directory, then it is bound to be a race.  And the
"locked directory" thing certainly appears to be the problem.  But apart
from that, it seems that the winner of the race is easy to predict, at
least going by from my limited runs (say 5 or 10 with sleep and 5 or so
without) as I can reproduce the problem by removing the sleep and work
around it by adding the sleep.

>>> I think the cause is
>>> that some process lingers with CWD in the _build directory which some
>>> other process tries to remove.  On MSYS, it's not possible to remove a
>>> directory that is "in use" like that.
>>>
>>> Ugly patch coming up shortly.
>>
>> As promised.
>>
>> Cheers,
>> Peter
>>
>> From 8ba67ab9885c8987d7a09ad650bcda69675b8285 Mon Sep 17 00:00:00 2001
>> From: Peter Rosin <address@hidden>
>> Date: Tue, 10 Jan 2012 00:14:08 +0100
>> Subject: [PATCH] dist: try to avoid race on MSYS when removing the distdir
>>
>> Fixes automake bug#10470.
>>
>> lib/am/distdir.am (am__remove_distdir): Add a short sleep to make old
>> processes release the directory before an attempt to remove it is made.
>> On MSYS, it is not possible to remove a directory that is "in use".
>> ---
>>  lib/am/distdir.am |    5 ++++-
>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/lib/am/distdir.am b/lib/am/distdir.am
>> index d8c1a89..e56d33b 100644
>> --- a/lib/am/distdir.am
>> +++ b/lib/am/distdir.am
>> @@ -22,10 +22,13 @@ if %?TOPDIR_P%
>>  distdir = $(PACKAGE)-$(VERSION)
>>  top_distdir = $(distdir)
>>  
>> +## On MSYS (1.0.17) it is not possible to remove a directory that is
>> +## in use, hence the sleep call, which gives processes time to exit
>> +## and "release" the directory before it is removed.
>>  am__remove_distdir = \
>>    { test ! -d "$(distdir)" \
>>      || { find "$(distdir)" -type d ! -perm -200 -exec chmod u+w {} ';' \
>> -         && rm -fr "$(distdir)"; }; }
>> +         && sleep 1 && rm -fr "$(distdir)"; }; }
>>  am__post_remove_distdir = $(am__remove_distdir)
>>  endif %?TOPDIR_P%
>>  
> Yuck.  As you've warned, this is really ugly indeed!  I don't think this 
> change
> is acceptable in its current form simply to pacify a minor[1] testsuite error.

No problem.

> Before going ahead, I'd like to have a better understanding of the reason of
> the failure you are seeing[2] -- even better, a testcase that embodies this
> understanding by making the issue consistently reproducible.
> 
> I will give a shot at this later or tomorrow, unless someone beats me.
> 
> Thanks,
>   Stefano
> 
> -*-*-
> 
> [1] Two reasons for which I see the error as minor:
>      - failing to remove the $(distdir) right away is not a serious error, 
> just
>        a minor annoyance, and easily fixed by a hand-issued "rm -rf";
>      - IMHO, one shouldn't be using a "peculiar" system like MSYS to build 
> real
>        distribution tarballs anyway.
> 
> [2] Now that I think of it, my knee-jerk guess is that the "( sleep 1 ) &"
>     put in configure by AM_SANITY_CHECK is the likely culprit.

I tried that hypothesis and bumped this to "( sleep 2 ) &", and the test
case failed even with my ugly "sleep 1" workaround.  I then bumped "my" fix
to "sleep 2" and the test case succeeded again.  So they certainly seem
connected.

Cheers,
Peter