bug#10470: MSYS: race in directory access?

[Peter Rosin]

Stefano Lattarini skrev 2012-01-12 10:15:
> On 01/11/2012 08:27 PM, Peter Rosin wrote:
>> Stefano Lattarini skrev 2012-01-11 18:31:
>>> Hi Peter, sorry for the delay.
>>
>> No rush!
>>
>>>>>>> We could enhance your original workaround like this:
>>>>>>>
>>>>>>>  am__remove_distdir = \
>>>>>>>    { test ! -d "$(distdir)" \
>>>>>>>      || { find "$(distdir)" -type d ! -perm -200 -exec chmod u+w {} ';' 
>>>>>>> \
>>>>>>> -         && rm -fr "$(distdir)"; }; }
>>>>>>> +         && if rm -fr "$(distdir)"; then :; else \
>>>>>>> +## On MSYS (1.0.17) it is not possible to remove a directory that is
>>>>>>> +## in use; so, if the first rm fails, we sleep some seconds and retry,
>>>>>>> +## to give pending processes some time to exit and "release" the
>>>>>>> +## directory before we removed.  See automake bug#10470.
>>>>>>> +              sleep 5 && rm -fr "$(distdir)"; fi; }; }
>>>>>>>  am__post_remove_distdir = $(am__remove_distdir)
>>>>>>>  endif %?TOPDIR_P%
>>>>>
>>>>>> This works, best so far!  Committable, if you ask me.
>>>>>>
>>>>
>>> Could you try the attached test case to see if it can reliably expose the
>>> problem on MSYS/MinGW?  If yes, I'll prepare a patch shortly.
>>
>> If I (try to) mend the race (the "rm -rf foo.d" reliably beats the "cd foo.d"
>> in the subshell) by adding a "sleep 1" before the "rm -rf foo.d"
>>
> Thanks for the info, I've amend the test accordingly.
> 
>> So, I guess no, the probelm is not exposed by the test.
>>
>> Did you mean "$my_sleep &" in the foo.test script?
>>
> Yes, I did; sorry for the sloppiness.
> 
>> If add that &, the test fails in much the same way as we've seen previously
>> in this bug report.
>>
> Good!
> 
> Attached is the proposed patch series: the first patch should expose the bug,
> the second patch should fix it.  I will apply them once I have confirmation
> the bug is correctly exposed and fixed.

After digging out that the patches are for maint, I managed to commit them,
but FYI this bugfix is only needed on master.  AM_SANITY_CHECK is not
aggressive enough on maint to cause problems, as can be seen below.  But ok
to commit wherever, it behaves as designed. Thanks!

I should also add that it's a POSIX violation to assume that directory
removal works when the dir is the CWD of a process.

        "If the directory is the root directory or the current working
        directory of any process, it is unspecified whether the function
        succeeds, or whether it shall fail and set errno to [EBUSY]."

Cheers,
Peter


On maint, with 1st patch:

$ make check TESTS="distcheck-pr10470.test distcheck-configure-flags-am.test"
XFAIL: distcheck-pr10470.test
PASS: distcheck-configure-flags-am.test

On maint, with both patches:

$ make check TESTS="distcheck-pr10470.test distcheck-configure-flags-am.test"
PASS: distcheck-pr10470.test
PASS: distcheck-configure-flags-am.test

Merging 1st patch into master:

$ make check TESTS="distcheck-pr10470.test distcheck-configure-flags-am.test"
XFAIL: distcheck-pr10470.test
FAIL: distcheck-configure-flags-am.test

Merging also the 2st patch into master:

$ make check TESTS="distcheck-pr10470.test distcheck-configure-flags-am.test"
PASS: distcheck-pr10470.test
PASS: distcheck-configure-flags-am.test