for review: bug fix for concurrent writes to `.d' files

[Fergus Henderson]

On 27-Feb-1998, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> 	The fix is to write the dependency file to `<foo>.d.tmp' and
> 	then rename the `.d.tmp' to `.d'.  This ensures that the update
> 	to the `.d' file is atomic. 

Well, gee, now we have a concurrent write problem for
the `.d.tmp' files.  Doh!

Obviously we need to use io__tepnam.  Let me try that again. 

[Testing this revised change revealed bugs in io__tmpnam;
I'll commit the fixes for those as a separate change.]

--------------------

Estimated hours taken: 1

compiler/modules.m:
	Fix a bug with parallel makes where the `.d' files were being
	stuffed up because two different invocations of the Mercury
	compiler were writing to them at the same time. 
	The problem occurs because the `.d' file is written out
	both when creating the `.c' file and when creating the
	`.trans_opt' file, and these steps might happen in parallel.

	The fix is to write the dependency file to a unique temporary
	file name and then rename it to the desired `.d' file name when
	we're done.  This ensures that the update to the `.d' file is atomic. 
	[In theory this is not a 100% portable solution, because
	rename() might not be atomic, but in practice it should be
	fine, because rename() is atomic on all the platforms of
	interest (i.e. all the ones that support parallel GNU make).]

	An alternative fix would be to not write out the `.d' files
	when creating `.trans_opt' files.  This would also be more
	efficient.  However, the danger is that the `.d' files might
	not be updated often enough.  I'm not really sure exactly
	how often they need to be updated, but the fix using
	renaming seems safer.

cvs diff  compiler/modules.m
Index: compiler/modules.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/modules.m,v
retrieving revision 1.56
diff -u -r1.56 modules.m
--- modules.m	1998/02/07 09:55:32	1.56
+++ modules.m	1998/02/27 12:25:19
@@ -509,11 +509,19 @@
 		MaybeTransOptDeps) -->
 	globals__io_lookup_bool_option(verbose, Verbose),
 	{ string__append(ModuleName, ".d", DependencyFileName) },
+	%
+	% To avoid problems with concurrent updates of `.d' files
+	% during parallel makes, we first create the file with a
+	% temporary name, and then rename it to the desired name
+	% when we've finished.
+	%
+	{ dir__dirname(DependencyFileName, DirName) },
+	io__tmpnam(DirName, "tmp_d", TmpDependencyFileName),
 	maybe_write_string(Verbose, "% Writing auto-dependency file `"),
 	maybe_write_string(Verbose, DependencyFileName),
 	maybe_write_string(Verbose, "'..."),
 	maybe_flush_output(Verbose),
-	io__open_output(DependencyFileName, Result),
+	io__open_output(TmpDependencyFileName, Result),
 	( { Result = ok(DepStream) } ->
 		{ set__list_to_set(LongDeps0, LongDepsSet0) },
 		{ set__delete(LongDepsSet0, ModuleName, LongDepsSet) },
@@ -661,10 +669,22 @@
 		]),
 
 		io__close_output(DepStream),
-		maybe_write_string(Verbose, " done.\n")
+		io__remove_file(DependencyFileName, _Result2),
+		io__rename_file(TmpDependencyFileName, DependencyFileName,
+			Result3),
+		( { Result3 = error(Error) } ->
+			{ io__error_message(Error, ErrorMsg) },
+			{ string__append_list(["can't rename file `",
+				TmpDependencyFileName, "' as `",
+				DependencyFileName, "': ", ErrorMsg],
+				Message) },
+			report_error(Message)
+		;
+			maybe_write_string(Verbose, " done.\n")
+		)
 	;
-		{ string__append_list(["can't open file `", DependencyFileName,
-				"' for output."], Message) },
+		{ string__append_list(["can't open file `",
+			TmpDependencyFileName, "' for output."], Message) },
 		report_error(Message)
 	).
 

-- 
Fergus Henderson <fjh at cs.mu.oz.au>   |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>   |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3         |     -- the last words of T. S. Garp.