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.

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