diff: bug fixes for io__make_temp

Fergus Henderson fjh at cs.mu.OZ.AU
Sat Jun 27 18:45:48 AEST 1998


On 25-Jun-1998, Thomas Charles CONWAY <conway at cs.mu.OZ.AU> wrote:
> Fergus Henderson, you write:
> > Fix race condition problems with io__tmpnam.
> > 
> > library/io.m:
> > 	Add io__make_temp, which is like io__tmpnam except that it
> > 	actually creates the file rather than just choosing its name.
> > 	This avoids a race condition where some other process could
> > 	create a file with the same name in between the call to
> > 	io__tmpnam and the call to io__open_output.
> > 	Add `pragma obsolete' declarations for io__tmpnam (both versions)
> > 	and document that their use is deprecated.
> > 	Reimplement io__tmpnam by calling io__make_temp and then
> > 	io__remove_file.
> > 
> > 	Also, use six hex digits rather than 3 decimal ones, and
> > 	add `1 << num_tries' rather than `1' to the counter each time.
> > 	This should hopefully reduce contention.
> > 
> > 	The race condition was being triggered when you started up
> > 	multiple processes each of which created multiple temp files
> > 	(e.g. when doing `mmake -j8 depend' in the various `tests'
> > 	subdirectories).
> 
> This looks fine, thanks.

Unfortunately the nightly tests revealed that it wasn't.
Hence the following changes:

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

Fix bugs in `io__make_temp', and improve error messages
in cases of I/O failures.

library/io.m:
	Fix two bugs in `io__make_temp':
		- it was calling open() but not calling close()
		- it was passing a value of type `long' to printf()
		  with a specified of `%X' instead of `%lX'
	Improve the error messages for I/O failures in all the
	predicates that open files, read bytes or characters, etc.

compiler/modules.m:
compiler/termination.m:
compiler/unused_args.m:
	If file opens fail, print out the error message returned by
	io.m.  Previously the code didn't print that out because
	io.m didn't return anything useful.

compiler/trans_opt.m:
	Fix a minor mistake in an error message: s/'/`/

Index: compiler/modules.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/modules.m,v
retrieving revision 1.78
diff -u -r1.78 modules.m
--- modules.m	1998/06/25 04:38:19	1.78
+++ modules.m	1998/06/27 08:11:04
@@ -921,14 +921,17 @@
 	maybe_write_string(Verbose, "'... "),
 	maybe_flush_output(Verbose),
 	io__open_output(OutputFileName, Result),
-	( { Result = ok(OutputStream) } ->
+	( { Result = ok(OutputStream) },
 		io__write_string(OutputStream, "\n"),
 		io__close_output(OutputStream),
 		maybe_write_string(Verbose, " done.\n")
-	;
+	; { Result = error(IOError) },
+		{ io__error_message(IOError, IOErrorMessage) },
 		io__write_string("\nError opening `"),
 		io__write_string(OutputFileName),
-		io__write_string("' for output\n")
+		io__write_string("'for output: "),
+		io__write_string(IOErrorMessage),
+		io__write_string(".\n")
 	).
 
 %-----------------------------------------------------------------------------%
@@ -1256,7 +1259,15 @@
 	maybe_write_string(Verbose, "'..."),
 	maybe_flush_output(Verbose),
 	io__open_output(TmpDependencyFileName, Result),
-	( { Result = ok(DepStream) } ->
+	( { Result = error(IOError) },
+		maybe_write_string(Verbose, " failed.\n"),
+		maybe_flush_output(Verbose),
+		{ io__error_message(IOError, IOErrorMessage) },
+		{ string__append_list(["error opening temporary file `",
+			TmpDependencyFileName, "' for output: ",
+			IOErrorMessage], Message) },
+		report_error(Message)
+	; { Result = ok(DepStream) },
 		{ list__append(IntDeps, ImplDeps, LongDeps0) },
 		{ ShortDeps0 = IndirectDeps },
 		{ set__list_to_set(LongDeps0, LongDepsSet0) },
@@ -1494,6 +1505,8 @@
 		io__rename_file(TmpDependencyFileName, DependencyFileName,
 			Result3),
 		( { Result3 = error(Error) } ->
+			maybe_write_string(Verbose, " failed.\n"),
+			maybe_flush_output(Verbose),
 			{ io__error_message(Error, ErrorMsg) },
 			{ string__append_list(["can't rename file `",
 				TmpDependencyFileName, "' as `",
@@ -1503,10 +1516,6 @@
 		;
 			maybe_write_string(Verbose, " done.\n")
 		)
-	;
-		{ string__append_list(["can't open file `",
-			TmpDependencyFileName, "' for output."], Message) },
-		report_error(Message)
 	).
 
 maybe_read_dependency_file(ModuleName, MaybeTransOptDeps) -->
@@ -1520,7 +1529,7 @@
 		maybe_write_string(Verbose, "'..."),
 		maybe_flush_output(Verbose),
 		io__open_input(DependencyFileName, OpenResult),
-		( { OpenResult = ok(Stream) } ->
+		( { OpenResult = ok(Stream) },
 			io__set_input_stream(Stream, OldStream),
 			module_name_to_file_name(ModuleName, ".trans_opt_date",
 				no, TransOptDateFileName0),
@@ -1538,15 +1547,18 @@
 				{ MaybeTransOptDeps = no }
 			),
 			io__set_input_stream(OldStream, _),	
-			io__close_input(Stream)
-		;
-			{ string__append_list(["can't open file `", 
-				DependencyFileName,
-				"' for input."], Message) },
-			{ MaybeTransOptDeps = no },
-			report_error(Message)
-		),
-		maybe_write_string(Verbose, " done.\n")
+			io__close_input(Stream),
+			maybe_write_string(Verbose, " done.\n")
+		; { OpenResult = error(IOError) },
+			maybe_write_string(Verbose, " failed.\n"),
+			maybe_flush_output(Verbose),
+			{ io__error_message(IOError, IOErrorMessage) },
+			{ string__append_list(["error opening file `", 
+				DependencyFileName, "' for input: ",
+				IOErrorMessage], Message) },
+			report_error(Message),
+			{ MaybeTransOptDeps = no }
+		)
 	;
 		{ MaybeTransOptDeps = no }
 	).
@@ -1745,16 +1757,20 @@
 		module_name_to_file_name(Module, ".order", yes, OrdFileName),
 		maybe_write_string(Verbose, "% Creating module order file `"),
 		maybe_write_string(Verbose, OrdFileName),
-		maybe_write_string(Verbose, "'...\n"),
+		maybe_write_string(Verbose, "'..."),
 		io__open_output(OrdFileName, OrdResult),
-		( { OrdResult = ok(OrdStream) } ->
+		( { OrdResult = ok(OrdStream) },
 			io__write_list(OrdStream, DepsOrdering, "\n\n", 
 					write_module_scc(OrdStream)),
 			io__close_output(OrdStream),
-			maybe_write_string(Verbose, "% done.\n")
-		;
-			{ string__append_list(["can't open file `", 
-	    			OrdFileName, "' for output."], OrdMessage) },
+			maybe_write_string(Verbose, " done.\n")
+		; { OrdResult = error(IOError) },
+			maybe_write_string(Verbose, " failed.\n"),
+			maybe_flush_output(Verbose),
+			{ io__error_message(IOError, IOErrorMessage) },
+			{ string__append_list(["error opening file `", 
+	    			OrdFileName, "' for output: ", IOErrorMessage],
+				OrdMessage) },
 			report_error(OrdMessage)
 		)
 	;
@@ -1930,14 +1946,17 @@
 	maybe_write_string(Verbose, DepFileName),
 	maybe_write_string(Verbose, "'...\n"),
 	io__open_output(DepFileName, DepResult),
-	( { DepResult = ok(DepStream) } ->
+	( { DepResult = ok(DepStream) },
 		generate_dep_file(SourceFileName, ModuleName, DepsMap,
 			DepStream),
 		io__close_output(DepStream),
 		maybe_write_string(Verbose, "% done.\n")
-	;
-		{ string__append_list(["can't open file `", DepFileName,
-			"' for output."], DepMessage) },
+	; { DepResult = error(IOError) },
+		maybe_write_string(Verbose, " failed.\n"),
+		maybe_flush_output(Verbose),
+		{ io__error_message(IOError, IOErrorMessage) },
+		{ string__append_list(["error opening file `", DepFileName,
+			"' for output: ", IOErrorMessage], DepMessage) },
 		report_error(DepMessage)
 	).
 
Index: compiler/termination.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/termination.m,v
retrieving revision 1.12
diff -u -r1.12 termination.m
--- termination.m	1998/05/25 21:48:56	1.12
+++ termination.m	1998/06/27 07:43:27
@@ -674,15 +674,16 @@
 	{ module_info_name(Module, ModuleName) },
 	module_name_to_file_name(ModuleName, ".opt.tmp", no, OptFileName),
 	io__open_append(OptFileName, OptFileRes),
-	( { OptFileRes = ok(OptFile) } ->
+	( { OptFileRes = ok(OptFile) },
 		io__set_output_stream(OptFile, OldStream),
 		termination__make_opt_int_preds(PredIds, Module),
 		io__set_output_stream(OldStream, _),
 		io__close_output(OptFile)
-	;
+	; { OptFileRes = error(IOError) },
 		% failed to open the .opt file for processing
-		io__write_strings(["Cannot open `",
-			OptFileName, "' for output\n"]),
+		{ io__error_message(IOError, IOErrorMessage) },
+		io__write_strings(["Error opening file `",
+			OptFileName, "' for output: ", IOErrorMessage]),
 		io__set_exit_status(1)
 	).
 
Index: compiler/trans_opt.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/trans_opt.m,v
retrieving revision 1.10
diff -u -r1.10 trans_opt.m
--- trans_opt.m	1998/05/29 08:50:48	1.10
+++ trans_opt.m	1998/06/27 07:44:15
@@ -92,7 +92,7 @@
 		io__progname_base("trans_opt.m", ProgName),
 		io__write_string(ProgName),
 		io__write_string(
-			": cannot open transitive optimisation file '"),
+			": cannot open transitive optimisation file `"),
 		io__write_string(TmpOptName),
 		io__write_string("' \n"),
 		io__write_string(ProgName),
Index: compiler/unused_args.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/unused_args.m,v
retrieving revision 1.50
diff -u -r1.50 unused_args.m
--- unused_args.m	1998/06/09 02:14:59	1.50
+++ unused_args.m	1998/06/27 07:45:11
@@ -101,11 +101,12 @@
 		module_name_to_file_name(ModuleName, ".opt.tmp", no,
 				OptFileName),
 		io__open_append(OptFileName, OptFileRes),
-		( { OptFileRes = ok(OptFile) } ->
+		( { OptFileRes = ok(OptFile) },
 			{ MaybeOptFile = yes(OptFile) }
-		;
-			io__write_strings(["Cannot open `",
-				OptFileName, "' for output\n"]),
+		; { OptFileRes = error(IOError) },
+			{ io__error_message(IOError, IOErrorMessage) },
+			io__write_strings(["Cannot open `", OptFileName,
+				"' for output: ", IOErrorMessage]),
 			io__set_exit_status(1),
 			{ MaybeOptFile = no }
 		)
Index: library/io.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/library/io.m,v
retrieving revision 1.157
diff -u -r1.157 io.m
--- io.m	1998/06/25 04:38:25	1.157
+++ io.m	1998/06/27 08:27:28
@@ -1113,38 +1113,38 @@
 	io__input_stream(Stream),
 	io__read_char(Stream, Result).
 
-io__read_char(Stream, Result, IO_0, IO) :-
-	io__read_char_code(Stream, Code, IO_0, IO),
+io__read_char(Stream, Result) -->
+	io__read_char_code(Stream, Code),
 	(
-		Code = -1
+		{ Code = -1 }
 	->
-		Result = eof
+		{ Result = eof }
 	;
-		char__to_int(Char, Code)
+		{ char__to_int(Char, Code) }
 	->
-		Result = ok(Char)
+		{ Result = ok(Char) }
 	;
-		% XXX improve error message
-		Result = error("read error")
+		io__make_err_msg("read failed: ", Msg),
+		{ Result = error(Msg) }
 	).
 
 io__read_byte(Result) -->
 	io__binary_input_stream(Stream),
 	io__read_byte(Stream, Result).
 
-io__read_byte(Stream, Result, IO_0, IO) :-
-	io__read_char_code(Stream, Code, IO_0, IO),
+io__read_byte(Stream, Result) -->
+	io__read_char_code(Stream, Code),
 	(
-		Code = -1
+		{ Code >= 0 }
 	->
-		Result = eof
+		{ Result = ok(Code) }
 	;
-		Code = -2
+		{ Code = -1 }
 	->
-		% XXX improve error message
-		Result = error("read error")
+		{ Result = eof }
 	;
-		Result = ok(Code)
+		io__make_err_msg("read failed: ", Msg),
+		{ Result = error(Msg) }
 	).
 
 io__read_word(Result) -->
@@ -1349,6 +1349,19 @@
 	ML_maybe_make_err_msg(RetVal != 0, ""read failed: "", RetStr);
 }").
 
+% io__make_err_msg(MessagePrefix, Message):
+%	`Message' is an error message obtained by looking up the
+%	message for the current value of errno and prepending
+%	`MessagePrefix'.
+:- pred io__make_err_msg(string, string, io__state, io__state).
+:- mode io__make_err_msg(in, out, di, uo) is det.
+
+:- pragma c_code(make_err_msg(Msg0::in, Msg::out, _IO0::di, _IO::uo),
+		will_not_call_mercury,
+"{
+	ML_maybe_make_err_msg(TRUE, Msg0, Msg);
+}").
+
 %-----------------------------------------------------------------------------%
 
 :- pred io__stream_file_size(stream, int, io__state, io__state).
@@ -1952,8 +1965,8 @@
 		{ Result = ok(NewStream) },
 		io__insert_stream_name(NewStream, FileName)
 	;
-		% XXX improve error message
-		{ Result = error("can't open input file") }
+		io__make_err_msg("can't open input file: ", Msg),
+		{ Result = error(Msg) }
 	).
 
 io__open_output(FileName, Result) -->
@@ -1962,8 +1975,8 @@
 		{ Result = ok(NewStream) },
 		io__insert_stream_name(NewStream, FileName)
 	;
-		% XXX improve error message
-		{ Result = error("can't open output file") }
+		io__make_err_msg("can't open output file: ", Msg),
+		{ Result = error(Msg) }
 	).
 
 io__open_append(FileName, Result) -->
@@ -1972,8 +1985,8 @@
 		{ Result = ok(NewStream) },
 		io__insert_stream_name(NewStream, FileName)
 	;
-		% XXX improve error message
-		{ Result = error("can't append to file") }
+		io__make_err_msg("can't append to file: ", Msg),
+		{ Result = error(Msg) }
 	).
 
 io__open_binary_input(FileName, Result) -->
@@ -1982,8 +1995,8 @@
 		{ Result = ok(NewStream) },
 		io__insert_stream_name(NewStream, FileName)
 	;
-		% XXX improve error message
-		{ Result = error("can't open input file") }
+		io__make_err_msg("can't open input file: ", Msg),
+		{ Result = error(Msg) }
 	).
 
 io__open_binary_output(FileName, Result) -->
@@ -1992,8 +2005,8 @@
 		{ Result = ok(NewStream) },
 		io__insert_stream_name(NewStream, FileName)
 	;
-		% XXX improve error message
-		{ Result = error("can't open output file") }
+		io__make_err_msg("can't open output file: ", Msg),
+		{ Result = error(Msg) }
 	).
 
 io__open_binary_append(FileName, Result) -->
@@ -2002,8 +2015,8 @@
 		{ Result = ok(NewStream) },
 		io__insert_stream_name(NewStream, FileName)
 	;
-		% XXX improve error message
-		{ Result = error("can't append to file") }
+		io__make_err_msg("can't append to file: ", Msg),
+		{ Result = error(Msg) }
 	).
 
 %-----------------------------------------------------------------------------%
@@ -2059,8 +2072,8 @@
 		io__set_output_stream(Stream, _),
 		{ Result = ok }
 	;
-		% XXX improve error message
-		{ Result = error("can't open output file") }
+		io__make_err_msg("can't open output file: ", Msg),
+		{ Result = error(Msg) }
 	).
 
 io__told_binary -->
@@ -2074,8 +2087,8 @@
 		io__set_binary_output_stream(Stream, _),
 		{ Result = ok }
 	;
-		% XXX improve error message
-		{ Result = error("can't open output file") }
+		io__make_err_msg("can't open output file: ", Msg),
+		{ Result = error(Msg) }
 	).
 
 %-----------------------------------------------------------------------------%
@@ -2339,6 +2352,7 @@
 
 void 		mercury_init_io(void);
 MercuryFile*	mercury_open(const char *filename, const char *type);
+void		mercury_fatal_io_error(void);
 int		mercury_output_error(MercuryFile* mf);
 void		mercury_print_string(MercuryFile* mf, const char *s);
 void		mercury_print_binary_string(MercuryFile* mf, const char *s);
@@ -2386,13 +2400,23 @@
 
 :- pragma(c_code, "
 
+void
+mercury_fatal_io_error(void)
+{
+	fatal_error(""cannot recover from I/O error -- execution terminated"");
+}
+
+").
+
+:- pragma(c_code, "
+
 int
 mercury_output_error(MercuryFile* mf)
 {
 	fprintf(stderr,
 		""Mercury runtime: error writing to output file: %s\\n"",
 		strerror(errno));
-	exit(1);
+	mercury_fatal_io_error();
 }
 
 ").
@@ -2453,7 +2477,7 @@
 			fprintf(stderr,
 				""Mercury runtime: error closing file: %s\\n"",
 				strerror(errno));
-			exit(1);
+			mercury_fatal_io_error();
 		}
 		oldmem(mf);
 	}
@@ -2465,7 +2489,7 @@
 
 :- pragma(c_code, io__read_char_code(File::in, CharCode::out, IO0::di, IO::uo),
 "
-	CharCode = mercury_getc((MercuryFile*)File);
+	CharCode = mercury_getc((MercuryFile *) File);
 	update_io(IO0, IO);
 ").
 
@@ -2973,7 +2997,7 @@
 	** Uses `open(..., O_CREATE | O_EXCL, ...)' to create the file,
 	** checking that there was no existing file with that name.
 	*/
-	int	len, err, num_tries;
+	int	len, err, fd, num_tries;
 	char	countstr[256];
 
 	len = strlen(Dir) + 1 + 5 + 3 + 1 + 3 + 1;
@@ -2985,20 +3009,30 @@
 	}
 	num_tries = 0;
 	do {
-		sprintf(countstr, ""%06X"", ML_io_tempnam_counter & 0xffffff);
+		sprintf(countstr, ""%06lX"", ML_io_tempnam_counter & 0xffffffL);
 		strcpy(FileName, Dir);
 		strcat(FileName, ""/"");
 		strncat(FileName, Prefix, 5);
 		strncat(FileName, countstr, 3);
 		strcat(FileName, ""."");
 		strncat(FileName, countstr + 3, 3);
-		err = open(FileName, O_WRONLY | O_CREAT | O_EXCL, 0600);
+		fd = open(FileName, O_WRONLY | O_CREAT | O_EXCL, 0600);
 		num_tries++;
 		ML_io_tempnam_counter += (1 << num_tries);
-	} while (err == -1 && errno == EEXIST &&
+	} while (fd == -1 && errno == EEXIST &&
 		num_tries < MAX_TEMPNAME_TRIES);
-	if (err == -1) {
-		fatal_error(""unable to create temporary file"");
+	if (fd == -1) {
+		fprintf(stderr, ""Mercury runtime: ""
+			""error opening temporary file: %s\\n"",
+			strerror(errno));
+		mercury_fatal_io_error();
+	} 
+	err = close(fd);
+	if (err != 0) {
+		fprintf(stderr, ""Mercury runtime: ""
+			""error closing temporary file: %s\\n"",
+			strerror(errno));
+		mercury_fatal_io_error();
 	}
 	update_io(IO0, IO);
 }").


-- 
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.



More information about the developers mailing list