From ecd339d1b4225627ecb371ed66d43139766fd4a8 Mon Sep 17 00:00:00 2001 From: Tuncer Ayaz Date: Fri, 15 Jan 2010 17:42:09 +0100 Subject: Don't return a false {error,eexist} in filelib:ensure_dir/1 This is about the non-atomicity of filelib:ensure_dir/1. When using filelib:ensure_dir/1 from multiple processes to create the same path or parts of the same directory structure (which happens with rebar's worker processes) it happens quite a lot that between a file:read_file_info/1 and file:make_dir/1 one of the other procs has already created the directory we want to create. mkdir(1) says one of the following for -p depending on which Unix like system you're on: "no error if existing" "no error will be reported if a directory given as an operand already exists" I've seen more than one Erlang project where the return value of ensure_dir/1 is ignored completely. To eliminate the race condition, call file:make_dir/1 without first testing whether the directory exists. If it succeeds everything is fine. Otherwise, if the error code is {error,eexists}, check whether the directory exists. If it does, everything is fine; if not, return {error,eexist} (which indicates that there exists a regular file with the same name, or (more unlikely) that another process removed the directory after the call to file:make_dir/1). Signed-off-by: Tuncer Ayaz --- lib/stdlib/src/filelib.erl | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'lib/stdlib') diff --git a/lib/stdlib/src/filelib.erl b/lib/stdlib/src/filelib.erl index d65588f0d1..3acc40ea20 100644 --- a/lib/stdlib/src/filelib.erl +++ b/lib/stdlib/src/filelib.erl @@ -228,7 +228,17 @@ ensure_dir(F) -> ok; false -> ensure_dir(Dir), - file:make_dir(Dir) + case file:make_dir(Dir) of + {error,eexist}=EExist -> + case do_is_dir(Dir, file) of + true -> + ok; + false -> + EExist + end; + Err -> + Err + end end. -- cgit v1.2.3 From 67657eddd662b2367b36a336d38c674e477e1184 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Sat, 23 Jan 2010 10:52:32 +0100 Subject: filelib_SUITE: strenghten tests of filelib:ensure_dir/1 Test that filelib:ensure_dir/1 returns {error,eexist} if there already exists a regular file with the name as a directory that should be created. While at it, slightly strenghten the otp_5960/1 test case by repeating each call to filelib:ensure_dir/1. --- lib/stdlib/test/filelib_SUITE.erl | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) (limited to 'lib/stdlib') diff --git a/lib/stdlib/test/filelib_SUITE.erl b/lib/stdlib/test/filelib_SUITE.erl index c9c6054f7b..d6abc1fba3 100644 --- a/lib/stdlib/test/filelib_SUITE.erl +++ b/lib/stdlib/test/filelib_SUITE.erl @@ -21,7 +21,7 @@ -export([all/1,init_per_testcase/2,fin_per_testcase/2, wildcard_one/1,wildcard_two/1,wildcard_errors/1, - fold_files/1,otp_5960/1]). + fold_files/1,otp_5960/1,ensure_dir_eexist/1]). -import(lists, [foreach/2]). @@ -38,7 +38,8 @@ fin_per_testcase(_Case, Config) -> ok. all(suite) -> - [wildcard_one,wildcard_two,wildcard_errors,fold_files,otp_5960]. + [wildcard_one,wildcard_two,wildcard_errors,fold_files,otp_5960, + ensure_dir_eexist]. wildcard_one(Config) when is_list(Config) -> ?line {ok,OldCwd} = file:get_cwd(), @@ -223,7 +224,9 @@ otp_5960(Config) when is_list(Config) -> ?line Name1 = filename:join(Dir, name1), ?line Name2 = filename:join(Dir, name2), ?line ok = filelib:ensure_dir(Name1), % parent is created + ?line ok = filelib:ensure_dir(Name1), % repeating it should be OK ?line ok = filelib:ensure_dir(Name2), % parent already exists + ?line ok = filelib:ensure_dir(Name2), % repeating it should be OK ?line Name3 = filename:join(Name1, name3), ?line {ok, FileInfo} = file:read_file_info(Dir), case os:type() of @@ -239,3 +242,16 @@ otp_5960(Config) when is_list(Config) -> ?line ok = file:write_file_info(Dir, #file_info{mode=Mode}), ok end. + +ensure_dir_eexist(Config) when is_list(Config) -> + ?line PrivDir = ?config(priv_dir, Config), + ?line Dir = filename:join(PrivDir, ensure_dir_eexist), + ?line Name = filename:join(Dir, "same_name_as_file_and_dir"), + ?line ok = filelib:ensure_dir(Name), + ?line ok = file:write_file(Name, <<"some string\n">>), + + %% There already is a file with the name of the directory + %% we want to create. + ?line NeedFile = filename:join(Name, "file"), + ?line {error, eexist} = filelib:ensure_dir(NeedFile), + ok. -- cgit v1.2.3