diff --git a/README.md b/README.md index e2e031d..28f3134 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,7 @@ following Mix config: # config/config.exs config :briefly, directory: [{:system, "TMPDIR"}, {:system, "TMP"}, {:system, "TEMP"}, "/tmp"], + directory_mode: 0o755, default_prefix: "briefly", default_extname: "" ``` diff --git a/lib/briefly/entry.ex b/lib/briefly/entry.ex index 6cf3d40..a617c9e 100644 --- a/lib/briefly/entry.ex +++ b/lib/briefly/entry.ex @@ -41,13 +41,34 @@ defmodule Briefly.Entry do @doc false def cleanup(pid) do case :ets.lookup(@dir_table, pid) do - [{^pid, _tmp}] -> + [{^pid, tmp}] -> + path_entries = :ets.lookup(@path_table, pid) + + secondaries = + Enum.reduce( + path_entries, + MapSet.new(), + fn path_entry = {_pid, path_value}, seen -> + delete_path(path_entry) + if path_value.original_owner != pid do + MapSet.put(seen, %{path_value | path: nil}) + else + seen + end + end + ) + + File.rmdir(tmp) + + Enum.each(secondaries, fn %{original_owner: owner_pid, root_dir: dir} -> + if !pid_registered?(owner_pid) do + File.rmdir(dir) + end + end) + + :ets.delete(@path_table, pid) :ets.delete(@dir_table, pid) - - entries = :ets.lookup(@path_table, pid) - Enum.each(entries, &delete_path/1) - - for {_pid, path} <- entries, do: path + for {_pid, path} <- path_entries, do: path [] -> [] @@ -58,19 +79,19 @@ defmodule Briefly.Entry do def give_away(path, to_pid, from_pid) when is_binary(path) and is_pid(to_pid) and is_pid(from_pid) do with true <- pid_registered?(from_pid), - true <- path_owner?(from_pid, path) do + path_entry when not is_nil(path_entry) <- path_entry_if_owner(from_pid, path) do if pid_registered?(to_pid) do - :ets.insert(@path_table, {to_pid, path}) - :ets.delete_object(@path_table, {from_pid, path}) + :ets.insert(@path_table, {to_pid, path_entry}) + :ets.delete_object(@path_table, {from_pid, path_entry}) :ok else server = server() {:ok, tmps} = GenServer.call(server, :roots) {:ok, tmp} = generate_tmp_dir(tmps) - :ok = GenServer.call(server, {:give_away, to_pid, tmp, path}) + :ok = GenServer.call(server, {:give_away, to_pid, tmp, path_entry}) - :ets.delete_object(@path_table, {from_pid, path}) + :ets.delete_object(@path_table, {from_pid, path_entry}) :ok end @@ -101,12 +122,12 @@ defmodule Briefly.Entry do {:reply, {:ok, dirs}, dirs} end - def handle_call({:give_away, pid, tmp, path}, _from, dirs) do + def handle_call({:give_away, pid, tmp, path_entry}, _from, dirs) do # Since we are writing on behalf of another process, we need to make sure # the monitor and writing to the tables happen within the same operation. Process.monitor(pid) :ets.insert_new(@dir_table, {pid, tmp}) - :ets.insert(@path_table, {pid, path}) + :ets.insert(@path_table, {pid, path_entry}) {:reply, :ok, dirs} end @@ -123,6 +144,17 @@ defmodule Briefly.Entry do def terminate(_reason, _state) do folder = fn entry, :ok -> delete_path(entry) end :ets.foldl(folder, :ok, @path_table) + + dir_folder = fn {_key, dir}, :ok -> + case File.rmdir(dir) do + :ok -> :ok + {:error, :eexist} -> :ok + {:error, :enoent} -> :ok + {:error, :enotdir} -> :ok + end + end + + :ets.foldl(dir_folder, :ok, @dir_table) end ## Helpers @@ -146,20 +178,33 @@ defmodule Briefly.Entry do end defp generate_tmp_dir(tmp_roots) do - {mega, _, _} = :os.timestamp() - subdir = "briefly-#{mega}" + subdir = path(%{prefix: "briefly-", extname: ""}) - if tmp = Enum.find_value(tmp_roots, &write_tmp_dir(Path.join(&1, subdir))) do + if tmp = Enum.find_value(tmp_roots, &write_tmp_dir({&1, subdir})) do {:ok, tmp} else {:error, %Briefly.NoRootDirectoryError{tmp_dirs: tmp_roots}} end end - defp write_tmp_dir(path) do - case File.mkdir_p(path) do - :ok -> path - {:error, _} -> nil + defp write_tmp_dir({root, path}) do + fullpath = Path.join(root, path) + + case File.mkdir_p(root) do + {:error, _} -> + nil + + :ok -> + case File.mkdir(fullpath) do + {:error, _} -> + nil + + :ok -> + case File.chmod(fullpath, Briefly.Config.directory_mode()) do + {:error, _} -> nil + :ok -> fullpath + end + end end end @@ -168,7 +213,8 @@ defmodule Briefly.Entry do case File.mkdir_p(path) do :ok -> - :ets.insert(@path_table, {self(), path}) + value = %{path: path, root_dir: tmp, original_owner: self()} + :ets.insert(@path_table, {self(), value}) {:ok, path} {:error, reason} when reason in [:eexist, :eacces] -> @@ -185,7 +231,8 @@ defmodule Briefly.Entry do case File.open(path, [:read, :write, :exclusive]) do {:ok, device_pid} -> - :ets.insert(@path_table, {self(), path}) + value = %{path: path, root_dir: tmp, original_owner: self()} + :ets.insert(@path_table, {self(), value}) {:ok, path, device_pid} {:error, reason} when reason in [:eexist, :eacces] -> @@ -207,7 +254,8 @@ defmodule Briefly.Entry do case :file.write_file(path, "", [:write, :raw, :exclusive, :binary]) do :ok -> - :ets.insert(@path_table, {self(), path}) + value = %{path: path, root_dir: tmp, original_owner: self()} + :ets.insert(@path_table, {self(), value}) {:ok, path} {:error, reason} when reason in [:eexist, :eacces] -> @@ -223,20 +271,21 @@ defmodule Briefly.Entry do {:error, last_error} end - defp path(options, tmp) do + defp path(options) do time = :erlang.monotonic_time() |> to_string |> String.trim("-") - folder = - Enum.join( - [ - prefix(options), - time, - random_padding() - ], - "-" - ) <> extname(options) - - Path.join([tmp, folder]) + Enum.join( + [ + prefix(options), + time, + random_padding() + ], + "-" + ) <> extname(options) + end + + defp path(options, tmp) do + Path.join([tmp, path(options)]) end defp random_padding(length \\ 20) do @@ -256,12 +305,15 @@ defmodule Briefly.Entry do :ets.member(@dir_table, pid) end - defp path_owner?(pid, path) do + defp path_entry_if_owner(pid, path) do owned_paths = :ets.lookup(@path_table, pid) - Enum.any?(owned_paths, fn {_pid, p} -> p == path end) + + Enum.find_value(owned_paths, fn {_pid, %{path: p} = entry} -> + if p == path, do: entry + end) end - defp delete_path({_pid, path}) do + defp delete_path({_pid, %{path: path}}) do File.rm_rf(path) :ok end diff --git a/mix.exs b/mix.exs index 7edfec9..4377b7c 100644 --- a/mix.exs +++ b/mix.exs @@ -56,6 +56,7 @@ defmodule Briefly.Mixfile do defp default_env do [ directory: [{:system, "TMPDIR"}, {:system, "TMP"}, {:system, "TEMP"}, "/tmp"], + directory_mode: 0o755, default_prefix: "briefly", default_extname: "" ] diff --git a/test/lib/briefly_test.exs b/test/lib/briefly_test.exs index bca192b..1554cea 100644 --- a/test/lib/briefly_test.exs +++ b/test/lib/briefly_test.exs @@ -320,6 +320,131 @@ defmodule Test.Briefly do refute File.exists?(path1) end end + + test "cleans up gifted temporary directories when appropriate" do + parent = self() + + {receive1_pid, receive1_ref} = + spawn_monitor(fn -> + receive do + :exit -> nil + end + end) + + {receive2_pid, receive2_ref} = + spawn_monitor(fn -> + receive do + :exit -> nil + end + end) + + {give_pid, give_ref} = + spawn_monitor(fn -> + {:ok, path1} = Briefly.create() + send(parent, {:path1, path1}) + File.open!(path1) + + {:ok, path2} = Briefly.create() + send(parent, {:path2, path2}) + File.open!(path2) + + :ok = Briefly.give_away(path1, receive1_pid) + :ok = Briefly.give_away(path2, receive1_pid) + + send(parent, :given_away) + + receive do + :continue -> :ok + end + + {:ok, path3} = Briefly.create() + send(parent, {:path3, path3}) + File.open!(path3) + + :ok = Briefly.give_away(path3, receive2_pid) + end) + + path1 = + receive do + {:path1, path} -> path + after + 1_000 -> flunk("didn't get a path") + end + + path2 = + receive do + {:path2, path} -> path + after + 1_000 -> flunk("didn't get a path") + end + + receive do + :given_away -> nil + after + 1_000 -> flunk("didn't get signal for given_away") + end + + send(receive1_pid, :exit) + + receive do + {:DOWN, ^receive1_ref, :process, ^receive1_pid, :normal} -> + # force sync by creating file in unknown process + parent = self() + + spawn(fn -> + {:ok, _} = Briefly.create() + send(parent, :continue) + end) + + receive do + :continue -> :ok + end + + cleanup_wait_loop(receive1_pid) + + refute File.exists?(path1) + refute File.exists?(path2) + assert File.exists?(Path.dirname(path1)) + end + + send(give_pid, :continue) + + path3 = + receive do + {:path3, path} -> path + after + 1_000 -> flunk("didn't get a path") + end + + receive do + {:DOWN, ^give_ref, :process, ^give_pid, :normal} -> + {:ok, _} = Briefly.create() + + assert File.exists?(path3) + end + + send(receive2_pid, :exit) + + receive do + {:DOWN, ^receive2_ref, :process, ^receive2_pid, :normal} -> + # force sync by creating file in unknown process + parent = self() + + spawn(fn -> + {:ok, _} = Briefly.create() + send(parent, :continue) + end) + + receive do + :continue -> :ok + end + + cleanup_wait_loop(receive2_pid) + + refute File.exists?(path3) + refute File.exists?(Path.dirname(path3)) + end + end end test "returns an io device if device is requested" do