From eb619c04765ef31b0e88329cbfd138d24558776e Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Thu, 1 Oct 2009 13:21:57 -0700 Subject: Avoid a small window when a pid file can be empty There's always been a small window of opportunity for a script to do File.read(pid).to_i would cause File.read() to read an empty file and return "". This closes that window while hopefully retaining backwards compatibility... We've always checked for dirname(pid) writability in Configurator, so we can safely write to a temporary file in the intended directory and then atomically rename() it to the destination path. --- lib/unicorn.rb | 16 +++++++++++----- test/exec/test_exec.rb | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/unicorn.rb b/lib/unicorn.rb index 1d22442..c5a0677 100644 --- a/lib/unicorn.rb +++ b/lib/unicorn.rb @@ -182,11 +182,17 @@ module Unicorn end unlink_pid_safe(pid) if pid - # not using O_EXCL here since we may be (intentionally) clobbering an - # existing file. Avoiding /all/ possible race conditions with the pid - # file is hard so we don't unlink it before writing nor do we rename() - # since that's clobbering, too... - File.open(path, 'wb') { |fp| fp.syswrite("#$$\n") } if path + if path + fp = begin + tmp = "#{File.dirname(path)}/#{rand}.#$$" + File.open(tmp, File::RDWR|File::CREAT|File::EXCL, 0644) + rescue Errno::EEXIST + retry + end + fp.syswrite("#$$\n") + File.rename(fp.path, path) + fp.close + end self.set_pid(path) end diff --git a/test/exec/test_exec.rb b/test/exec/test_exec.rb index e753c2d..268a84e 100644 --- a/test/exec/test_exec.rb +++ b/test/exec/test_exec.rb @@ -728,7 +728,7 @@ end } } sleep 1 # racy - daemon_pid = pid_file.read.to_i + daemon_pid = File.read(pid_file.path).to_i assert daemon_pid > 0 Process.kill(:HUP, daemon_pid) sleep 1 # racy -- cgit v1.2.3-24-ge0c7