Subject: Re: bug#10470: MSYS: race in directory access?

From: Stefano Lattarini <[email-masked]>
Date: Thu, 12 Jan 2012 10:15:36 +0100

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.

Thanks,
  Stefano.
>From 657eed2e1b1f2e0fac19f4c840b72a71ec6e1141 Mon Sep 17 00:00:00 2001
Message-Id: <657eed2e1[email-masked]>
From: Stefano Lattarini <[email-masked]>
Date: Thu, 12 Jan 2012 10:06:14 +0100
Subject: [PATCH 1/2] coverage: expose automake bug#10470 (distcheck-related)

* tests/distcheck-pr10470.test: New test.
* tests/Makefile.am (TESTS, XFAIL_TESTS): Add it.

Report and suggestions by Peter Rosin and Eric Blake.
---
 tests/Makefile.am            |    2 +
 tests/distcheck-pr10470.test |   61 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 0 deletions(-)
 create mode 100755 tests/distcheck-pr10470.test

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 8817b64..e10dbd5 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -23,6 +23,7 @@ auxdir2.test \
 cond17.test \
 dist-auxfile.test \
 dist-auxfile-2.test \
+distcheck-pr10470.test \
 gcj6.test \
 java-nobase.test \
 objext-pr10128.test \
@@ -352,6 +353,7 @@ distcheck-hook2.test \
 distcheck-missing-m4.test \
 distcheck-outdated-m4.test \
 distcheck-pr9579.test \
+distcheck-pr10470.test \
 distcheck-override-infodir.test \
 dmalloc.test \
 doc-parsing-buglets-colneq-subst.test \
diff --git a/tests/distcheck-pr10470.test b/tests/distcheck-pr10470.test
new file mode 100755
index 0000000..e774dd7
--- /dev/null
+++ b/tests/distcheck-pr10470.test
@@ -0,0 +1,61 @@
+#! /bin/sh
+# Copyright (C) 2012 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Ensure "make distcheck" does not experience racy failures on
+# systems (like MinGW/MSYS) that cannot remove a directory "in use"
+# by a process (e.g., that is its "current working directory").
+# See automake bug#10470.
+
+parallel_tests=no
+. ./defs || Exit 1
+
+set -e
+
+mkdir foo.d
+sh -c "cd foo.d && sleep '4'" &
+# Without this sleep, the "rm -rf foo.d" below would reliably beat
+# the "cd foo.d" in the subshell above, and the test would be always
+# skipped, even on MinGW/MSYS.
+sleep '1'
+rm -rf foo.d && skip_ 'system is able to remove "in use" directories'
+
+echo AC_OUTPUT >> configure.in
+
+cat > Makefile.am <<END
+TESTS = foo.test
+EXTRA_DIST= foo.test
+END
+
+cat > foo.test <<END
+#!/bin/sh
+sleep '4' &
+exit 0
+END
+chmod a+x foo.test
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE
+./configure
+
+# We can build the distribution.
+$MAKE distcheck >output 2>&1 || { cat output; Exit 1; }
+cat output
+# Sanity check: verify that our code has hit a problem removing
+# the distdir, but has recovered from it.
+grep "rm:.*$destdir" output || fatal_ "expected code path not covered"
+
+:
-- 
1.7.7.3

>From 49cca731a596c9224281eb34114aeee6795dab10 Mon Sep 17 00:00:00 2001
Message-Id: <49cca731a[email-masked]>
In-Reply-To: <657eed2e1[email-masked]>
References: <657eed2e1[email-masked]>
From: Stefano Lattarini <[email-masked]>
Date: Wed, 11 Jan 2012 18:09:07 +0100
Subject: [PATCH 2/2] dist: avoid $(distdir) removal failure on MSYS/MinGW

Fixes automake big#10470.

* lib/am/distdir.am (am__remove_distdir): On MSYS (1.0.17) it is
not possible to remove a directory that is in use; so, if rm fails,
we sleep some seconds and retry, to give pending processes some
time to exit and "release" the directory.
* tests/Makefile.am (XFAIL_TESTS): Remove 'distcheck-pr10470.test'.

Report and suggestions by Peter Rosin and Eric Blake.
---
 lib/am/distdir.am |   17 ++++++++++++-----
 tests/Makefile.am |    1 -
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/lib/am/distdir.am b/lib/am/distdir.am
index a681101..cd2e4bf 100644
--- a/lib/am/distdir.am
+++ b/lib/am/distdir.am
@@ -1,6 +1,6 @@
 ## automake - create Makefile.in from Makefile.am
 ## Copyright (C) 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009,
-## 2010, 2011 Free Software Foundation, Inc.
+## 2010, 2011, 2012 Free Software Foundation, Inc.
 
 ## This program is free software; you can redistribute it and/or modify
 ## it under the terms of the GNU General Public License as published by
@@ -23,10 +23,17 @@ distdir = $(PACKAGE)-$(VERSION)
 top_distdir = $(distdir)
 
 am__remove_distdir = \
-  { test ! -d "$(distdir)" \
-    || { find "$(distdir)" -type d ! -perm -200 -exec chmod u+w {} ';' \
-         && rm -fr "$(distdir)"; }; }
-
+  if test -d "$(distdir)"; then \
+    find "$(distdir)" -type d ! -perm -200 -exec chmod u+w {} ';' \
+      && rm -rf "$(distdir)" \
+## 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 remove it.  The value of "some seconds" is 5 for the moment,
+## which is mostly an arbitrary value, but seems high enough in practice.
+## See automake bug#10470.
+      || { sleep 5 && rm -rf "$(distdir)"; }; \
+  else :; fi
 endif %?TOPDIR_P%
 
 if %?SUBDIRS%
diff --git a/tests/Makefile.am b/tests/Makefile.am
index e10dbd5..e79c3be 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -23,7 +23,6 @@ auxdir2.test \
 cond17.test \
 dist-auxfile.test \
 dist-auxfile-2.test \
-distcheck-pr10470.test \
 gcj6.test \
 java-nobase.test \
 objext-pr10128.test \
-- 
1.7.7.3