[m-rev.] for review: Fix resource leaks in dir fold predicates.

Peter Wang novalazy at gmail.com
Mon May 12 16:38:26 AEST 2014


The directory stream was not closed if dir.open succeeded in opening
the directory, but failing to read the first entry (which may be as
simple as an empty directory).

The directory stream was also not closed when dir.foldl2_process_dir2
returns an error value.

library/dir.m:
	As above.

tests/hard_coded/Mmakefile:
tests/hard_coded/dir_fold.m:
	Add regression test.

diff --git a/library/dir.m b/library/dir.m
index bbef641..19c4de9 100644
--- a/library/dir.m
+++ b/library/dir.m
@@ -1368,9 +1368,9 @@ dir.foldl2_process_dir(SymLinkParent, P, DirName, ParentIds0, Recursive,
                 ),
                 (
                     ExcpResult = succeeded({DirRes, Continue, Dir}),
+                    dir.close(Dir, CleanupRes, !IO),
                     (
                         DirRes = ok(T),
-                        dir.close(Dir, CleanupRes, !IO),
                         (
                             CleanupRes = ok,
                             Result = ok(T)
@@ -1782,7 +1782,16 @@ dir.check_dir_readable(DirName, IsReadable, Result, !IO) :-
     "ML_dir_read_first_entry").
 
 dir.read_first_entry(Dir, Result, !IO) :-
-    dir.read_entry(Dir, Result, !IO).
+    dir.read_entry(Dir, Result, !IO),
+    (
+        Result = ok(_)
+    ;
+        ( Result = eof
+        ; Result = error(_)
+        ),
+        % Close the directory stream immediately to avoid resource leaks.
+        dir.close(Dir, _, !IO)
+    ).
 
 :- pred make_win32_dir_open_result_ok(dir.stream::in, string::in,
     io.result({dir.stream, string})::out, io::di, io::uo) is det.
diff --git a/tests/hard_coded/Mmakefile b/tests/hard_coded/Mmakefile
index a020b68..cfa12e9 100644
--- a/tests/hard_coded/Mmakefile
+++ b/tests/hard_coded/Mmakefile
@@ -613,6 +613,7 @@ ifeq "$(findstring profdeep,$(GRADE))" ""
 		bitmap_empty \
 		bitmap_test \
 		bit_buffer_test \
+		dir_fold \
 		dir_test \
 		final_excp \
 		init_excp \
diff --git a/tests/hard_coded/dir_fold.m b/tests/hard_coded/dir_fold.m
new file mode 100644
index 0000000..14b6287
--- /dev/null
+++ b/tests/hard_coded/dir_fold.m
@@ -0,0 +1,69 @@
+%-----------------------------------------------------------------------------%
+
+:- module dir_fold.
+:- interface.
+
+:- import_module io.
+
+:- pred main(io::di, io::uo) is det.
+
+%-----------------------------------------------------------------------------%
+%-----------------------------------------------------------------------------%
+
+:- implementation.
+
+:- import_module bool.
+:- import_module dir.
+:- import_module int.
+:- import_module unit.
+
+%-----------------------------------------------------------------------------%
+
+main(!IO) :-
+    DirName = "empty",
+    dir.make_directory(DirName, ResMkdir, !IO),
+    (
+        ResMkdir = ok,
+        % dir.foldl2 used to leak a file descriptor on empty directories.
+        % Repeat enough times to make it apparent.  On Linux, the default
+        % maximum number of open files to a process is 1024.
+        repeat(DirName, 1025, !IO),
+        io.remove_file(DirName, _, !IO),
+        io.write_string("done.\n", !IO)
+    ;
+        ResMkdir = error(Error),
+        report_error(io.error_message(Error), !IO)
+    ).
+
+:- pred repeat(string::in, int::in, io::di, io::uo) is det.
+
+repeat(DirName, Count, !IO) :-
+    ( Count > 0 ->
+        dir.foldl2(nothing, DirName, unit, Res, !IO),
+        (
+            Res = ok(_ : unit),
+            repeat(DirName, Count - 1, !IO)
+        ;
+            Res = error(_, Error),
+            report_error(io.error_message(Error), !IO)
+        )
+    ;
+        true
+    ).
+
+:- pred nothing(string::in, string::in, io.file_type::in, bool::out,
+    unit::in, unit::out, io::di, io::uo) is det.
+
+nothing(_DirName, _BaseName, _FileType, Continue, !Data, !IO) :-
+    Continue = yes.
+
+:- pred report_error(string::in, io::di, io::uo) is det.
+
+report_error(Error, !IO) :-
+    io.stderr_stream(Stream, !IO),
+    io.write_string(Stream, Error, !IO),
+    io.nl(Stream, !IO),
+    io.set_exit_status(1, !IO).
+
+%-----------------------------------------------------------------------------%
+% vim: ft=mercury ts=4 sts=4 sw=4 et
-- 
1.8.4




More information about the reviews mailing list