automake-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

FYI: fix race condition in elisp's recover rule


From: Alexandre Duret-Lutz
Subject: FYI: fix race condition in elisp's recover rule
Date: Tue, 29 Mar 2005 20:46:41 +0200
User-agent: Gnus/5.110003 (No Gnus v0.3) Emacs/21.3.50 (gnu/linux)

I'm installing this on HEAD and branch-1-9.

2005-03-29  Alexandre Duret-Lutz  <address@hidden>

        * lib/am/lisp.am ($(am__ELCFILES)): Prevent races if the recover
        rule is run with `make -j'.
        * doc/automake.texi (Multiple Outputs): Adjust.
        * tests/lisp6.test: Augment it.
        * tests/lisp8.test: New file.
        * tests/Makefile.am (TESTS): Add lisp8.test.
        Suggested by Bruno Haible.

Index: doc/automake.texi
===================================================================
RCS file: /cvs/automake/automake/doc/automake.texi,v
retrieving revision 1.44.2.46
diff -u -r1.44.2.46 automake.texi
--- doc/automake.texi   27 Mar 2005 12:39:31 -0000      1.44.2.46
+++ doc/automake.texi   29 Mar 2005 18:44:24 -0000
@@ -8440,38 +8440,47 @@
 data.c: data.foo
         foo data.foo
 data.h: data.c
+## Recover from the removal of $@@
         @@if test -f $@@; then :; else \
           rm -f data.c; \
           $(MAKE) $(AM_MAKEFLAGS) data.c; \
         fi
 @end example
 
-The above scales easily to more outputs and more inputs.  One of the
-output is picked up to serve as a witness of the run of the command,
-it depends upon all inputs, and all other outputs depend upon it.  For
-instance if @command{foo} should additionally read @file{data.bar} and
-also produce @file{data.w} and @file{data.x}, we would write:
+The above scheme can be extended to handle more outputs and more
+inputs.  One of the output is picked up to serve as a witness of the
+run of the command, it depends upon all inputs, and all other outputs
+depend upon it.  For instance if @command{foo} should additionally
+read @file{data.bar} and also produce @file{data.w} and @file{data.x},
+we would write:
 
 @example
 data.c: data.foo data.bar
         foo data.foo data.bar
 data.h data.w data.x: data.c
+## Recover from the removal of $@@
         @@if test -f $@@; then :; else \
           rm -f data.c; \
           $(MAKE) $(AM_MAKEFLAGS) data.c; \
         fi
 @end example
 
-There is still a minor problem with this setup.  @command{foo} outputs
-four files, but we do not know in which order these files are created.
-Suppose that @file{data.h} is created before @file{data.c}.  Then we
-have a weird situation.  The next time @command{make} is run,
address@hidden will appear older than @file{data.c}, the second rule
-will be triggered, a shell will be started to execute the
address@hidden@dots{}fi} command, but actually it will just execute the
address@hidden branch, that is: nothing.  In other words, because the
-witness we selected is not the first file created by @command{foo},
address@hidden will start a shell to do nothing each time it is run.
+However there are now two minor problems in this setup.  One is related
+to the timestamp ordering of @file{data.h}, @file{data.w},
address@hidden, and @file{data.c}.  The other one is a race condition
+if a parallel @command{make} attempts to run multiple instance of the
+recover block at once.
+
+Let us deal with the first problem.  @command{foo} outputs four files,
+but we do not know in which order these files are created.  Suppose
+that @file{data.h} is created before @file{data.c}.  Then we have a
+weird situation.  The next time @command{make} is run, @file{data.h}
+will appear older than @file{data.c}, the second rule will be
+triggered, a shell will be started to execute the @address@hidden
+command, but actually it will just execute the @code{then} branch,
+that is: nothing.  In other words, because the witness we selected is
+not the first file created by @command{foo}, @command{make} will start
+a shell to do nothing each time it is run.
 
 A simple riposte is to fix the timestamps when this happens.
 
@@ -8482,14 +8491,14 @@
         @@if test -f $@@; then \
           touch $@@; \
         else \
+## Recover from the removal of $@@
           rm -f data.c; \
           $(MAKE) $(AM_MAKEFLAGS) data.c; \
         fi
 @end example
 
-Another solution, not incompatible with the previous one, is to use a
-different and dedicated file as witness, rather than using any of
address@hidden's outputs.
+Another solution is to use a different and dedicated file as witness,
+rather than using any of @command{foo}'s outputs.
 
 @example
 data.stamp: data.foo data.bar
@@ -8498,9 +8507,8 @@
         foo data.foo data.bar
         @@mv -f data.tmp $@@
 data.c data.h data.w data.x: data.stamp
-        @@if test -f $@@; then \
-          touch $@@; \
-        else \
+## Recover from the removal of $@@
+        @@if test -f $@@; then :; else \
           rm -f data.stamp; \
           $(MAKE) $(AM_MAKEFLAGS) data.stamp; \
         fi
@@ -8511,6 +8519,44 @@
 renamed to @file{data.stamp} after @command{foo} has run, because we
 do not want to update @file{data.stamp} if @command{foo} fails.
 
+This solution still suffers from the second problem: the race
+condition in the recover rule.  If, after a successful build, a user
+erases @file{data.c} and @file{data.h}, and run @samp{make -j}, then
address@hidden may start both recover rules in parallel.  If the two
+instances of the rule execute @samp{$(MAKE) $(AM_MAKEFLAGS)
+data.stamp} concurrently the build is likely to fail (for instance the
+two rules will create @file{data.tmp}, but only one can rename it).
+
+Admittedly, such weird situation does not happen during ordinary
+builds.  It occurs only when the build tree is mutilated.  Here
address@hidden and @file{data.h} have been explicitly removed without
+also removing @file{data.stamp} and the other output files.
address@hidden clean; make} will always recover from these situations even
+with parallel makes, so you may decide that the recover rule is just
+an help to non-parallel make users and leave things as-is.  Fixing
+this requires some locking mechanism to ensure only one instance of
+the recover rule rebuild @code{data.stamp}.  One could imagine
+something along the following lines.
+
address@hidden
+data.c data.h data.w data.x: data.stamp
+## Recover from the removal of $@@
+        @@if test -f $@@; then :; else \
+          trap 'rm -rf data.lock data.stamp 1 2 13 15; \
+## mkdir is a portable test-and-set
+          if mkdir data.lock 2>/dev/null; then \
+## This code is being executed by the first process.
+            rm -f data.stamp; \
+            $(MAKE) $(AM_MAKEFLAGS) data.stamp; \
+          else \
+## This code is being executed by the follower processes.
+## Wait until the first process is done.
+            while test -d data.lock; do sleep 1; done; \
+## Succeed if and only if the first process succeeded.
+            test -f data.stamp; exit $$?; \
+        fi
address@hidden example
+
 Using a dedicated witness like this is very handy when the list of
 output files is not known beforehand.  As an illustration, consider
 the following rules to compile many @file{*.el} files into
@@ -8529,11 +8575,21 @@
         @@mv -f elc-temp $@@
 
 $(ELCFILES): elc-stamp
-        @@if test -f $@@; then \
-          touch $@@; \
-        else \
-          rm -f elc-stamp; \
-          $(MAKE) $(AM_MAKEFLAGS) elc-stamp; \
+## Recover from the removal of $@@
+        @@if test -f $@@; then :; else \
+          trap 'rm -rf elc-lock elc-stamp' 1 2 13 15; \
+          if mkdir elc-lock 2>/dev/null; then \
+## This code is being executed by the first process.
+            rm -f elc-stamp; \
+            $(MAKE) $(AM_MAKEFLAGS) elc-stamp; \
+            rmdir elc-lock; \
+          else \
+## This code is being executed by the follower processes.
+## Wait until the first process is done.
+            while test -d elc-lock; do sleep 1; done; \
+## Succeed if and only if the first process succeeded.
+            test -f elc-stamp; exit $$?; \
+          fi; \
         fi
 @end example
 
Index: doc/stamp-vti
===================================================================
RCS file: /cvs/automake/automake/doc/stamp-vti,v
retrieving revision 1.57.2.40
diff -u -r1.57.2.40 stamp-vti
--- doc/stamp-vti       27 Mar 2005 12:39:31 -0000      1.57.2.40
+++ doc/stamp-vti       29 Mar 2005 18:44:24 -0000
@@ -1,4 +1,4 @@
address@hidden UPDATED 27 March 2005
address@hidden UPDATED 29 March 2005
 @set UPDATED-MONTH March 2005
 @set EDITION 1.9.5a
 @set VERSION 1.9.5a
Index: doc/version.texi
===================================================================
RCS file: /cvs/automake/automake/doc/version.texi,v
retrieving revision 1.57.2.40
diff -u -r1.57.2.40 version.texi
--- doc/version.texi    27 Mar 2005 12:39:31 -0000      1.57.2.40
+++ doc/version.texi    29 Mar 2005 18:44:24 -0000
@@ -1,4 +1,4 @@
address@hidden UPDATED 27 March 2005
address@hidden UPDATED 29 March 2005
 @set UPDATED-MONTH March 2005
 @set EDITION 1.9.5a
 @set VERSION 1.9.5a
Index: lib/am/lisp.am
===================================================================
RCS file: /cvs/automake/automake/lib/am/lisp.am,v
retrieving revision 1.44.2.1
diff -u -r1.44.2.1 lisp.am
--- lib/am/lisp.am      16 Mar 2005 00:10:42 -0000      1.44.2.1
+++ lib/am/lisp.am      29 Mar 2005 18:44:24 -0000
@@ -49,12 +49,30 @@
 $(am__ELCFILES): elc-stamp
 ## Recover from the removal of address@hidden
 ##
-## Make sure not to call `make elc-stamp' if emacs is not available,
-## because as all *.elc files appear as missing, a parallel make would
-## attempt to build elc-stamp several times.
+## Do not call `make elc-stamp' if emacs is not available, because it would
+## be useless.
        @if test "$(EMACS)" != no && test ! -f $@; then \
-         rm -f elc-stamp; \
-         $(MAKE) $(AM_MAKEFLAGS) elc-stamp; \
+## If `make -j' is used and more than one file has been erased, several
+## processes can execute this block.  We have to make sure that only
+## the first one will run `$(MAKE) $(AM_MAKEFLAGS) elc-stamp', and the
+## other ones will wait.
+##
+## There is a race here if only one child of make receive a signal.
+## In that case the build may fail.  We remove elc-stamp when we receive
+## a signal so we are sure the build will succeed the next time.
+         trap 'rm -rf elc-lock elc-stamp' 1 2 13 15; \
+         if mkdir elc-lock 2>/dev/null; then \
+## This code is being executed by the first process.
+           rm -f elc-stamp; \
+           $(MAKE) $(AM_MAKEFLAGS) elc-stamp; \
+           rmdir elc-lock; \
+         else \
+## This code is being executed by the follower processes.
+## Wait until the first process is done.
+           while test -d elc-lock; do sleep 1; done; \
+## Succeed if and only if the first process succeeded.
+           test -f elc-stamp; exit $$?; \
+         fi; \
        else : ; fi
 
 
Index: tests/Makefile.am
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.am,v
retrieving revision 1.565.2.16
diff -u -r1.565.2.16 Makefile.am
--- tests/Makefile.am   16 Mar 2005 00:10:42 -0000      1.565.2.16
+++ tests/Makefile.am   29 Mar 2005 18:44:24 -0000
@@ -318,6 +318,7 @@
 lisp5.test \
 lisp6.test \
 lisp7.test \
+lisp8.test \
 listval.test \
 location.test \
 longline.test \
Index: tests/Makefile.in
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.in,v
retrieving revision 1.733.2.29
diff -u -r1.733.2.29 Makefile.in
--- tests/Makefile.in   16 Mar 2005 00:10:42 -0000      1.733.2.29
+++ tests/Makefile.in   29 Mar 2005 18:44:24 -0000
@@ -444,6 +444,7 @@
 lisp5.test \
 lisp6.test \
 lisp7.test \
+lisp8.test \
 listval.test \
 location.test \
 longline.test \
Index: tests/lisp6.test
===================================================================
RCS file: /cvs/automake/automake/tests/lisp6.test,v
retrieving revision 1.1
diff -u -r1.1 lisp6.test
--- tests/lisp6.test    1 Feb 2004 12:54:02 -0000       1.1
+++ tests/lisp6.test    29 Mar 2005 18:44:24 -0000
@@ -1,5 +1,5 @@
 #! /bin/sh
-# Copyright (C) 2004  Free Software Foundation, Inc.
+# Copyright (C) 2004, 2005  Free Software Foundation, Inc.
 #
 # This file is part of GNU Automake.
 #
@@ -83,6 +83,14 @@
 test -f am-three.elc
 test -f elc-stamp
 
+# Let's mutilate the source tree, the check the recover rule.
+rm -f am-*.elc
+$MAKE
+test -f am-one.elc
+test -f am-two.elc
+test -f am-three.elc
+test -f elc-stamp
+
 $MAKE install
 test -f lisp/am-one.el
 test -f lisp/am-one.elc
Index: tests/lisp8.test
===================================================================
RCS file: tests/lisp8.test
diff -N tests/lisp8.test
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/lisp8.test    29 Mar 2005 18:44:24 -0000
@@ -0,0 +1,65 @@
+#! /bin/sh
+# Copyright (C) 2005  Free Software Foundation, Inc.
+#
+# This file is part of GNU Automake.
+#
+# GNU Automake 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.
+#
+# GNU Automake 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 GNU Automake; see the file COPYING.  If not, write to
+# the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+# Boston, MA 02111-1307, USA.
+
+# Check the recover rule of lisp_LISP with parallel make.
+
+required='GNUmake emacs'
+. ./defs || exit 1
+
+set -e
+
+cat > Makefile.am << 'EOF'
+dist_lisp_LISP = am-one.el am-two.el am-three.el
+EOF
+
+cat >> configure.in << 'EOF'
+AM_PATH_LISPDIR
+AC_OUTPUT
+EOF
+
+echo "(require 'am-two)" > am-one.el
+echo "(require 'am-three) (provide 'am-two)" > am-two.el
+echo "(provide 'am-three)" > am-three.el
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE --add-missing
+./configure
+
+$MAKE -j >stdout
+
+cat stdout
+test 1 -eq `grep 'Warnings can be ignored' stdout | wc -l`
+
+test -f am-one.elc
+test -f am-two.elc
+test -f am-three.elc
+test -f elc-stamp
+
+rm -f am-*.elc
+
+$MAKE -j >stdout
+
+cat stdout
+test 1 -eq `grep 'Warnings can be ignored' stdout | wc -l`
+test -f am-one.elc
+test -f am-two.elc
+test -f am-three.elc
+test -f elc-stamp

-- 
Alexandre Duret-Lutz





reply via email to

[Prev in Thread] Current Thread [Next in Thread]