Skip to content
This repository
Browse code

Stop overwriting existing pidfiles.

A race condition can arise when two servers are started simultaneously. Both
instances may complete the check for an existing pidfile before either one
writes it.

Now the pidfile is opened with ::File::EXCL, which raises an error if the file
already exists. This error is handled by retrying the check and the write.
  • Loading branch information...
commit 85b50e2135db1c2773a3c1025b5813e523beb324 1 parent 42b11a7
Tim Moore authored January 30, 2013
5  lib/rack/server.rb
@@ -329,8 +329,11 @@ def daemonize_app
329 329
       end
330 330
 
331 331
       def write_pid
332  
-        ::File.open(options[:pid], 'w'){ |f| f.write("#{Process.pid}") }
  332
+        ::File.open(options[:pid], ::File::CREAT | ::File::EXCL | ::File::WRONLY ){ |f| f.write("#{Process.pid}") }
333 333
         at_exit { ::File.delete(options[:pid]) if ::File.exist?(options[:pid]) }
  334
+      rescue Errno::EEXIST
  335
+        check_pid!
  336
+        retry
334 337
       end
335 338
 
336 339
       def check_pid!
16  test/spec_server.rb
@@ -110,6 +110,22 @@ def with_stderr
110 110
     server.send(:pidfile_process_status).should.eql :not_owned
111 111
   end
112 112
 
  113
+  should "not write pid file when it is created after check" do
  114
+    pidfile = Tempfile.open('pidfile') { |f| break f }.path
  115
+    ::File.delete(pidfile)
  116
+    server = Rack::Server.new(:pid => pidfile)
  117
+    ::File.open(pidfile, 'w') { |f| f.write(1) }
  118
+    with_stderr do |err|
  119
+      should.raise(SystemExit) do
  120
+        server.send(:write_pid)
  121
+      end
  122
+      err.rewind
  123
+      output = err.read
  124
+      output.should.match(/already running/)
  125
+      output.should.include? pidfile
  126
+    end
  127
+  end
  128
+
113 129
   should "inform the user about existing pidfiles with running processes" do
114 130
     pidfile = Tempfile.open('pidfile') { |f| f.write(1); break f }.path
115 131
     server = Rack::Server.new(:pid => pidfile)

0 notes on commit 85b50e2

Please sign in to comment.
Something went wrong with that request. Please try again.