From 86fd8957c7799368619aa8ce054b440716494a11 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 19 Jul 2009 17:11:01 -0700 Subject: Remove core Tempfile dependency (1.9.2-preview1 compat) With the 1.9.2preview1 release (and presumably 1.9.1 p243), the Ruby core team has decided that bending over backwards to support crippled operating/file systems was necessary and that files must be closed before unlinking. Regardless, this is more efficient than using Tempfile because: 1) no delegation is necessary, this is a real File object 2) no mkdir is necessary for locking, we can trust O_EXCL to work properly without unnecessary FS activity 3) no finalizer is needed to unlink the file, we unlink it as soon as possible after creation. (cherry picked from commit 344b85ff28e160daa6563ab7c80b733abdeb874a) Conflicts: lib/unicorn.rb lib/unicorn/app/exec_cgi.rb lib/unicorn/tee_input.rb --- lib/unicorn.rb | 23 ++++++++++------------- lib/unicorn/app/exec_cgi.rb | 9 ++------- lib/unicorn/util.rb | 17 +++++++++++++++++ 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/lib/unicorn.rb b/lib/unicorn.rb index aac530b..49435d8 100644 --- a/lib/unicorn.rb +++ b/lib/unicorn.rb @@ -54,8 +54,7 @@ module Unicorn 0 => $0.dup, } - Worker = Struct.new(:nr, :tempfile) unless defined?(Worker) - class Worker + class Worker < Struct.new(:nr, :tmp) # worker objects may be compared to just plain numbers def ==(other_nr) self.nr == other_nr @@ -323,7 +322,7 @@ module Unicorn self.pid = @pid.chomp('.oldbin') if @pid proc_name 'master' else - worker = WORKERS.delete(pid) and worker.tempfile.close rescue nil + worker = WORKERS.delete(pid) and worker.tmp.close rescue nil logger.info "reaped #{status.inspect} " \ "worker=#{worker.nr rescue 'unknown'}" end @@ -383,16 +382,16 @@ module Unicorn end # forcibly terminate all workers that haven't checked in in @timeout - # seconds. The timeout is implemented using an unlinked tempfile + # seconds. The timeout is implemented using an unlinked File # shared between the parent process and each worker. The worker - # runs File#chmod to modify the ctime of the tempfile. If the ctime + # runs File#chmod to modify the ctime of the File. If the ctime # is stale for >@timeout seconds, then we'll kill the corresponding # worker. def murder_lazy_workers diff = stat = nil WORKERS.dup.each_pair do |pid, worker| stat = begin - worker.tempfile.stat + worker.tmp.stat rescue => e logger.warn "worker=#{worker.nr} PID:#{pid} stat error: #{e.inspect}" kill_worker(:QUIT, pid) @@ -416,9 +415,7 @@ module Unicorn SIG_QUEUE << :QUIT # forcibly emulate SIGQUIT return end - tempfile = Tempfile.new(nil) # as short as possible to save dir space - tempfile.unlink # don't allow other processes to find or see it - worker = Worker.new(worker_nr, tempfile) + worker = Worker.new(worker_nr, Unicorn::Util.tmpio) @before_fork.call(self, worker) pid = fork { worker_loop(worker) } WORKERS[pid] = worker @@ -466,10 +463,10 @@ module Unicorn proc_name "worker[#{worker.nr}]" START_CTX.clear init_self_pipe! - WORKERS.values.each { |other| other.tempfile.close! rescue nil } + WORKERS.values.each { |other| other.tmp.close rescue nil } WORKERS.clear LISTENERS.each { |sock| sock.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC) } - worker.tempfile.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC) + worker.tmp.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC) @after_fork.call(self, worker) # can drop perms @timeout /= 2.0 # halve it for select() build_app! unless @preload_app @@ -489,7 +486,7 @@ module Unicorn master_pid = Process.ppid # slightly racy, but less memory usage init_worker_process(worker) nr = 0 # this becomes negative if we need to reopen logs - alive = worker.tempfile # tempfile is our lifeline to the master process + alive = worker.tmp # tmp is our lifeline to the master process ready = LISTENERS t = ti = 0 @@ -555,7 +552,7 @@ module Unicorn begin Process.kill(signal, pid) rescue Errno::ESRCH - worker = WORKERS.delete(pid) and worker.tempfile.close rescue nil + worker = WORKERS.delete(pid) and worker.tmp.close rescue nil end end diff --git a/lib/unicorn/app/exec_cgi.rb b/lib/unicorn/app/exec_cgi.rb index 8f81d78..b0fbedc 100644 --- a/lib/unicorn/app/exec_cgi.rb +++ b/lib/unicorn/app/exec_cgi.rb @@ -42,11 +42,8 @@ module Unicorn::App # Calls the app def call(env) - out, err = Tempfile.new(''), Tempfile.new('') - out.unlink - err.unlink + out, err = Unicorn::Util.tmpio, Unicorn::Util.tmpio inp = force_file_input(env) - inp.sync = out.sync = err.sync = true pid = fork { run_child(inp, out, err, env) } inp.close pid, status = Process.waitpid2(pid) @@ -126,9 +123,7 @@ module Unicorn::App elsif inp.size == 0 # inp could be a StringIO or StringIO-like object ::File.open('/dev/null') else - tmp = Tempfile.new('') - tmp.unlink - tmp.binmode + tmp = Unicorn::Util.tmpio # Rack::Lint::InputWrapper doesn't allow sysread :( buf = '' diff --git a/lib/unicorn/util.rb b/lib/unicorn/util.rb index 2d3f827..d2214b7 100644 --- a/lib/unicorn/util.rb +++ b/lib/unicorn/util.rb @@ -1,4 +1,5 @@ require 'fcntl' +require 'tmpdir' module Unicorn class Util @@ -39,6 +40,22 @@ module Unicorn nr end + # creates and returns a new File object. The File is unlinked + # immediately, switched to binary mode, and userspace output + # buffering is disabled + def tmpio + fp = begin + File.open("#{Dir::tmpdir}/#{rand}", + File::RDWR|File::CREAT|File::EXCL, 0600) + rescue Errno::EEXIST + retry + end + File.unlink(fp.path) + fp.binmode + fp.sync = true + fp + end + end end -- cgit v1.2.3-24-ge0c7 From 864a25c20b1b7d785cdff20f99e4246e7a7f9a93 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 19 Jul 2009 16:48:12 -0700 Subject: fix tests to run correctly under 1.9.2preview1 test/test_helper doesn't seem to be required correctly anymore, since we know our own module/test names don't conflict, just fix RUBYLIB to include $(test_prefix) With test_util.rb, using #reopen with Tempfile objects seems prone to the objects being closed. Not completely sure what is going on but I'll just sidestep around it since I've stopped trusting Tempfile by now... (cherry picked from commit 344b85ff28e160daa6563ab7c80b733abdeb874a) --- GNUmakefile | 8 +++---- test/unit/test_util.rb | 58 ++++++++++++++++++++++++++------------------------ 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/GNUmakefile b/GNUmakefile index 1145143..47c6b5f 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -48,7 +48,7 @@ http11: lib/unicorn/http11.$(DLEXT) $(test_prefix)/.stamp: $(inst_deps) mkdir -p $(test_prefix)/.ccache - tar c bin ext lib GNUmakefile Manifest | (cd $(test_prefix) && tar x) + tar c `cat Manifest` | (cd $(test_prefix) && tar x) $(MAKE) -C $(test_prefix) clean $(MAKE) -C $(test_prefix) http11 shebang > $@ @@ -88,14 +88,14 @@ run_test = $(quiet_pre) setsid $(ruby) -w $(arg) $(TEST_OPTS) $(quiet_post) || \ %.n: arg = $(subst .n,,$(subst --, -n ,$@)) %.n: t = $(subst .n,$(log_suffix),$@) %.n: export PATH := $(test_prefix)/bin:$(PATH) -%.n: export RUBYLIB := $(test_prefix)/lib:$(RUBYLIB) +%.n: export RUBYLIB := $(test_prefix):$(test_prefix)/lib:$(RUBYLIB) %.n: $(test_prefix)/.stamp $(run_test) $(T): arg = $@ $(T): t = $(subst .rb,$(log_suffix),$@) $(T): export PATH := $(test_prefix)/bin:$(PATH) -$(T): export RUBYLIB := $(test_prefix)/lib:$(RUBYLIB) +$(T): export RUBYLIB := $(test_prefix):$(test_prefix)/lib:$(RUBYLIB) $(T): $(test_prefix)/.stamp $(run_test) @@ -141,7 +141,7 @@ $(T_r).%.r: rv = $(subst .r,,$(subst $(T_r).,,$@)) $(T_r).%.r: extra = ' 'v$(rv) $(T_r).%.r: arg = $(T_r) $(T_r).%.r: export PATH := $(test_prefix)/bin:$(PATH) -$(T_r).%.r: export RUBYLIB := $(test_prefix)/lib:$(RUBYLIB) +$(T_r).%.r: export RUBYLIB := $(test_prefix):$(test_prefix)/lib:$(RUBYLIB) $(T_r).%.r: export UNICORN_RAILS_TEST_VERSION = $(rv) $(T_r).%.r: export RAILS_GIT_REPO = $(CURDIR)/$(rails_git) $(T_r).%.r: $(test_prefix)/.stamp $(rails_git)/info/cloned-stamp diff --git a/test/unit/test_util.rb b/test/unit/test_util.rb index 1616eac..032f0be 100644 --- a/test/unit/test_util.rb +++ b/test/unit/test_util.rb @@ -43,20 +43,21 @@ class TestUtil < Test::Unit::TestCase tmp = Tempfile.new(nil) tmp_path = tmp.path.dup.freeze Encoding.list.each { |encoding| - tmp.reopen(tmp_path, "a:#{encoding.to_s}") - tmp.sync = true - assert_equal encoding, tmp.external_encoding - assert_nil tmp.internal_encoding - File.unlink(tmp_path) - assert ! File.exist?(tmp_path) - Unicorn::Util.reopen_logs - assert_equal tmp_path, tmp.path - assert File.exist?(tmp_path) - assert_equal tmp.stat.inspect, File.stat(tmp_path).inspect - assert_equal encoding, tmp.external_encoding - assert_nil tmp.internal_encoding - assert_equal(EXPECT_FLAGS, EXPECT_FLAGS & tmp.fcntl(Fcntl::F_GETFL)) - assert tmp.sync + File.open(tmp_path, "a:#{encoding.to_s}") { |fp| + fp.sync = true + assert_equal encoding, fp.external_encoding + assert_nil fp.internal_encoding + File.unlink(tmp_path) + assert ! File.exist?(tmp_path) + Unicorn::Util.reopen_logs + assert_equal tmp_path, fp.path + assert File.exist?(tmp_path) + assert_equal fp.stat.inspect, File.stat(tmp_path).inspect + assert_equal encoding, fp.external_encoding + assert_nil fp.internal_encoding + assert_equal(EXPECT_FLAGS, EXPECT_FLAGS & fp.fcntl(Fcntl::F_GETFL)) + assert fp.sync + } } end if STDIN.respond_to?(:external_encoding) @@ -66,20 +67,21 @@ class TestUtil < Test::Unit::TestCase Encoding.list.each { |ext| Encoding.list.each { |int| next if ext == int - tmp.reopen(tmp_path, "a:#{ext.to_s}:#{int.to_s}") - tmp.sync = true - assert_equal ext, tmp.external_encoding - assert_equal int, tmp.internal_encoding - File.unlink(tmp_path) - assert ! File.exist?(tmp_path) - Unicorn::Util.reopen_logs - assert_equal tmp_path, tmp.path - assert File.exist?(tmp_path) - assert_equal tmp.stat.inspect, File.stat(tmp_path).inspect - assert_equal ext, tmp.external_encoding - assert_equal int, tmp.internal_encoding - assert_equal(EXPECT_FLAGS, EXPECT_FLAGS & tmp.fcntl(Fcntl::F_GETFL)) - assert tmp.sync + File.open(tmp_path, "a:#{ext.to_s}:#{int.to_s}") { |fp| + fp.sync = true + assert_equal ext, fp.external_encoding + assert_equal int, fp.internal_encoding + File.unlink(tmp_path) + assert ! File.exist?(tmp_path) + Unicorn::Util.reopen_logs + assert_equal tmp_path, fp.path + assert File.exist?(tmp_path) + assert_equal fp.stat.inspect, File.stat(tmp_path).inspect + assert_equal ext, fp.external_encoding + assert_equal int, fp.internal_encoding + assert_equal(EXPECT_FLAGS, EXPECT_FLAGS & fp.fcntl(Fcntl::F_GETFL)) + assert fp.sync + } } } end if STDIN.respond_to?(:external_encoding) -- cgit v1.2.3-24-ge0c7 From 7bcfcde9f9bfed2ecd666869e4adb71ee1861ced Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 19 Jul 2009 17:17:32 -0700 Subject: app/exec_cgi: fix 1.9 compatibility "/dev/null" must be opened in binary mode for Rack-compliance. Additionally, avoid '' to create an empty string and use Unicorn::Z instead. --- lib/unicorn/app/exec_cgi.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/unicorn/app/exec_cgi.rb b/lib/unicorn/app/exec_cgi.rb index b0fbedc..861d5e6 100644 --- a/lib/unicorn/app/exec_cgi.rb +++ b/lib/unicorn/app/exec_cgi.rb @@ -121,12 +121,12 @@ module Unicorn::App if inp.respond_to?(:fileno) && Integer === inp.fileno inp elsif inp.size == 0 # inp could be a StringIO or StringIO-like object - ::File.open('/dev/null') + ::File.open('/dev/null', 'rb') else tmp = Unicorn::Util.tmpio # Rack::Lint::InputWrapper doesn't allow sysread :( - buf = '' + buf = Unicorn::Z.dup while inp.read(CHUNK_SIZE, buf) tmp.syswrite(buf) end -- cgit v1.2.3-24-ge0c7 From 018f90f0394e83e99aff543e7f4c1ae69f39a6fc Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 19 Jul 2009 17:25:47 -0700 Subject: unicorn 0.8.3 --- CHANGELOG | 1 + lib/unicorn/const.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index cc7d5fe..36a00f0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,4 @@ +v0.8.3 - Ruby 1.9.2 preview1 compatibility v0.8.2 - socket handling bugfixes and usability tweaks v0.8.1 - safer timeout handling, more consistent reload behavior v0.8.0 - enforce Rack dependency, minor performance improvements and fixes diff --git a/lib/unicorn/const.rb b/lib/unicorn/const.rb index 72a4d61..e9c6667 100644 --- a/lib/unicorn/const.rb +++ b/lib/unicorn/const.rb @@ -5,7 +5,7 @@ module Unicorn # gave about a 3% to 10% performance improvement over using the strings directly. # Symbols did not really improve things much compared to constants. module Const - UNICORN_VERSION="0.8.2".freeze + UNICORN_VERSION="0.8.3".freeze DEFAULT_HOST = "0.0.0.0".freeze # default TCP listen host address DEFAULT_PORT = "8080".freeze # default TCP listen port -- cgit v1.2.3-24-ge0c7 From 13c71ade9b8860e7cc49f86edb3490235fd1bce6 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Thu, 6 Aug 2009 15:42:41 -0700 Subject: http_response: pass through unknown status codes Rack::Utils::HTTP_STATUS_CODES does not define all status codes that are assigned by IANA nor does it handle experimental/unknown/ad-hoc status codes. So fall back to blindly accepting the status code as given by the application and hope it works. --- lib/unicorn/http_response.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/unicorn/http_response.rb b/lib/unicorn/http_response.rb index 15df3f6..3bf9347 100644 --- a/lib/unicorn/http_response.rb +++ b/lib/unicorn/http_response.rb @@ -37,7 +37,7 @@ module Unicorn # writes the rack_response to socket as an HTTP response def self.write(socket, rack_response) status, headers, body = rack_response - status = CODES[status.to_i] + status = CODES[status.to_i] || status OUT.clear # Don't bother enforcing duplicate supression, it's a Hash most of -- cgit v1.2.3-24-ge0c7 From e0e6bf48a899088bd4327ce5fd2c223ec7e826d5 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Thu, 6 Aug 2009 15:48:02 -0700 Subject: unicorn 0.8.4 --- CHANGELOG | 1 + lib/unicorn/const.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 36a00f0..54b02ff 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,4 @@ +v0.8.4 - pass through unknown HTTP status codes v0.8.3 - Ruby 1.9.2 preview1 compatibility v0.8.2 - socket handling bugfixes and usability tweaks v0.8.1 - safer timeout handling, more consistent reload behavior diff --git a/lib/unicorn/const.rb b/lib/unicorn/const.rb index e9c6667..6730367 100644 --- a/lib/unicorn/const.rb +++ b/lib/unicorn/const.rb @@ -5,7 +5,7 @@ module Unicorn # gave about a 3% to 10% performance improvement over using the strings directly. # Symbols did not really improve things much compared to constants. module Const - UNICORN_VERSION="0.8.3".freeze + UNICORN_VERSION="0.8.4".freeze DEFAULT_HOST = "0.0.0.0".freeze # default TCP listen host address DEFAULT_PORT = "8080".freeze # default TCP listen port -- cgit v1.2.3-24-ge0c7