diff options
| -rw-r--r-- | KNOWN_ISSUES | 20 | ||||
| -rw-r--r-- | Rakefile | 17 | ||||
| -rwxr-xr-x | bin/unicorn | 7 | ||||
| -rwxr-xr-x | bin/unicorn_rails | 27 | ||||
| -rw-r--r-- | ext/unicorn_http/unicorn_http.rl | 8 | ||||
| -rw-r--r-- | lib/unicorn.rb | 36 | ||||
| -rw-r--r-- | test/rails/test_rails.rb | 2 | ||||
| -rw-r--r-- | test/unit/test_http_parser_ng.rb | 4 | ||||
| -rw-r--r-- | test/unit/test_tee_input.rb | 32 | ||||
| -rw-r--r-- | test/unit/test_util.rb | 10 |
10 files changed, 110 insertions, 53 deletions
diff --git a/KNOWN_ISSUES b/KNOWN_ISSUES index e83e34e..c32d751 100644 --- a/KNOWN_ISSUES +++ b/KNOWN_ISSUES @@ -3,6 +3,14 @@ Occasionally odd {issues}[link:ISSUES.html] arise without a transparent or acceptable solution. Those issues are documented here. +* Under Ruby 1.9.1, methods like Array#shuffle and Array#sample will + segfault if called after forking. This is fixed in trunk (r26936) and + should be backported to the next 1.9.1 stable release (after p378). + Until then, it is advisable to call "Kernel.rand" in your after_fork + hook to reinitialize the random number generator. + + See http://redmine.ruby-lang.org/issues/show/2962 for more details + * When using "preload_app true", with apps using background threads need to restart them in the after_fork hook because threads are never shared with child processes. Additionally, any synchronization @@ -36,6 +44,18 @@ acceptable solution. Those issues are documented here. where the unicorn gem is installed (e.g. /usr/lib/ruby/gems/1.8/gems/unicorn-VERSION/lib) + With current versions of isolate, it is also recommended that you + disable it with the <tt>before_exec</tt> hook prevent the PATH and + RUBYOPT environment variable modifications from propagating between + upgrades in your Unicorn config file: + + before_exec do |server| + Isolate.disable + end + + Future versions (unreleased as of 2010.04.20) of isolate will not + require this as environment variable modifications will be idempotent. + * WONTFIX: code reloading and restarts with Sinatra 0.3.x (and likely older versions) apps is broken. The workaround is to force production mode to disable code reloading as well as disabling "run" in your @@ -175,16 +175,17 @@ end # optional rake-compiler support in case somebody needs to cross compile begin - spec = Gem::Specification.load('unicorn.gemspec') - require 'rake/extensiontask' - unless test ?r, "ext/unicorn_http/unicorn_http.c" - abort "run 'gmake ragel' or 'make ragel' to generate the Ragel source" - end mk = "ext/unicorn_http/Makefile" if test ?r, mk - abort "run 'gmake -C ext/unicorn_http clean' and " \ - "remove #{mk} before using rake-compiler" + warn "run 'gmake -C ext/unicorn_http clean' and\n" \ + "remove #{mk} before using rake-compiler" + else + unless test ?r, "ext/unicorn_http/unicorn_http.c" + abort "run 'gmake ragel' or 'make ragel' to generate the Ragel source" + end + spec = Gem::Specification.load('unicorn.gemspec') + require 'rake/extensiontask' + Rake::ExtensionTask.new('unicorn_http', spec) end - Rake::ExtensionTask.new('unicorn_http', spec) rescue LoadError end diff --git a/bin/unicorn b/bin/unicorn index 658d27c..beece97 100755 --- a/bin/unicorn +++ b/bin/unicorn @@ -5,8 +5,7 @@ require 'optparse' ENV["RACK_ENV"] ||= "development" daemonize = false -listeners = [] -options = { :listeners => listeners } +options = { :listeners => [] } host, port = Unicorn::Const::DEFAULT_HOST, Unicorn::Const::DEFAULT_PORT set_listener = false @@ -81,7 +80,7 @@ opts = OptionParser.new("", 24, ' ') do |opts| "listen on HOST:PORT or PATH", "this may be specified multiple times", "(default: #{Unicorn::Const::DEFAULT_LISTEN})") do |address| - listeners << address + options[:listeners] << address end opts.on("-c", "--config-file FILE", "Unicorn-specific config file") do |f| @@ -112,7 +111,7 @@ ru = ARGV[0] || "config.ru" abort "configuration file #{ru} not found" unless File.exist?(ru) app = Unicorn.builder(ru, opts) -listeners << "#{host}:#{port}" if set_listener +options[:listeners] << "#{host}:#{port}" if set_listener if $DEBUG require 'pp' diff --git a/bin/unicorn_rails b/bin/unicorn_rails index 37ee027..72ab288 100755 --- a/bin/unicorn_rails +++ b/bin/unicorn_rails @@ -5,8 +5,7 @@ require 'optparse' require 'fileutils' daemonize = false -listeners = [] -options = { :listeners => listeners } +options = { :listeners => [] } host, port = Unicorn::Const::DEFAULT_HOST, Unicorn::Const::DEFAULT_PORT set_listener = false ENV['RAILS_ENV'] ||= "development" @@ -70,7 +69,7 @@ opts = OptionParser.new("", 24, ' ') do |opts| "listen on HOST:PORT or PATH", "this may be specified multiple times", "(default: #{Unicorn::Const::DEFAULT_LISTEN})") do |address| - listeners << address + options[:listeners] << address end opts.on("-c", "--config-file FILE", "Unicorn-specific config file") do |f| @@ -108,14 +107,14 @@ opts = OptionParser.new("", 24, ' ') do |opts| opts.parse! ARGV end -config = ARGV[0] || (File.exist?('config.ru') ? 'config.ru' : nil) +ru = ARGV[0] || (File.exist?('config.ru') ? 'config.ru' : nil) -if config && config =~ /\.ru\z/ +if ru && ru =~ /\.ru\z/ # parse embedded command-line options in config.ru comments - /^#\\(.*)/ =~ File.read(config) and opts.parse!($1.split(/\s+/)) + /^#\\(.*)/ =~ File.read(ru) and opts.parse!($1.split(/\s+/)) end -def rails_builder(config, daemonize) +def rails_builder(ru, daemonize) # this lambda won't run until after forking if preload_app is false lambda do || # Load Rails and (possibly) the private version of Rack it bundles. @@ -125,7 +124,7 @@ def rails_builder(config, daemonize) abort "#$0 must be run inside RAILS_ROOT: #{err.inspect}" end - inner_app = case config + inner_app = case ru when nil require 'config/environment' @@ -146,12 +145,12 @@ def rails_builder(config, daemonize) ActionController::Dispatcher.new end when /\.ru$/ - raw = File.open(config, "rb") { |fp| fp.sysread(fp.stat.size) } + raw = File.read(ru) raw.sub!(/^__END__\n.*/, '') - eval("Rack::Builder.new {(#{raw}\n)}.to_app", nil, config) + eval("Rack::Builder.new {(#{raw}\n)}.to_app", TOPLEVEL_BINDING, ru) else - require config - Object.const_get(File.basename(config, '.rb').capitalize) + require ru + Object.const_get(File.basename(ru, '.rb').capitalize) end Rack::Builder.new do @@ -180,8 +179,8 @@ def rails_builder(config, daemonize) end end -app = rails_builder(config, daemonize) -listeners << "#{host}:#{port}" if set_listener +app = rails_builder(ru, daemonize) +options[:listeners] << "#{host}:#{port}" if set_listener if $DEBUG require 'pp' diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl index f0b602b..264db68 100644 --- a/ext/unicorn_http/unicorn_http.rl +++ b/ext/unicorn_http/unicorn_http.rl @@ -299,12 +299,8 @@ static void write_value(VALUE req, struct http_parser *hp, } action end_chunked_body { - if (HP_FL_TEST(hp, HASTRAILER)) { - HP_FL_SET(hp, INTRAILER); - cs = http_parser_en_Trailers; - } else { - cs = http_parser_first_final; - } + HP_FL_SET(hp, INTRAILER); + cs = http_parser_en_Trailers; ++p; assert(p <= pe && "buffer overflow after chunked body"); goto post_exec; diff --git a/lib/unicorn.rb b/lib/unicorn.rb index b63abeb..72cda10 100644 --- a/lib/unicorn.rb +++ b/lib/unicorn.rb @@ -1,8 +1,17 @@ # -*- encoding: binary -*- require 'fcntl' -require 'unicorn/socket_helper' require 'etc' +require 'unicorn/socket_helper' +require 'unicorn/const' +require 'unicorn/http_request' +require 'unicorn/configurator' +require 'unicorn/util' +require 'unicorn/tee_input' + +# autoload this so the app can prefer a different version, we +# don't rely on Rack itself for much and should be compatible for +# 1.0.x and 1.1.x+ autoload :Rack, 'rack' # Unicorn module containing all of the classes (include C extensions) for running @@ -17,12 +26,10 @@ module Unicorn class ClientShutdown < EOFError end - autoload :Const, 'unicorn/const' - autoload :HttpRequest, 'unicorn/http_request' + # we load HttpResponse last since it depends on Rack, and we + # want the application to be able to specify Rack (if they're + # *not* using config.ru) autoload :HttpResponse, 'unicorn/http_response' - autoload :Configurator, 'unicorn/configurator' - autoload :TeeInput, 'unicorn/tee_input' - autoload :Util, 'unicorn/util' class << self def run(app, options = {}) @@ -395,9 +402,12 @@ module Unicorn # machine) comes out of suspend/hibernation if (last_check + timeout) >= (last_check = Time.now) murder_lazy_workers + else + # wait for workers to wakeup on suspend + master_sleep(timeout/2.0 + 1) end maintain_worker_count if respawn - master_sleep + master_sleep(1) when :QUIT # graceful shutdown break when :TERM, :INT # immediate shutdown @@ -478,9 +488,9 @@ module Unicorn # wait for a signal hander to wake us up and then consume the pipe # Wake up every second anyways to run murder_lazy_workers - def master_sleep + def master_sleep(sec) begin - ready = IO.select([SELF_PIPE.first], nil, nil, 1) or return + ready = IO.select([SELF_PIPE.first], nil, nil, sec) or return ready.first && ready.first.first or return loop { SELF_PIPE.first.read_nonblock(Const::CHUNK_SIZE) } rescue Errno::EAGAIN, Errno::EINTR @@ -800,15 +810,15 @@ module Unicorn def build_app! if app.respond_to?(:arity) && app.arity == 0 - # exploit COW in case of preload_app. Also avoids race - # conditions in Rainbows! since load/require are not thread-safe - Unicorn.constants.each { |x| Unicorn.const_get(x) } - if defined?(Gem) && Gem.respond_to?(:refresh) logger.info "Refreshing Gem list" Gem.refresh end self.app = app.call + + # exploit COW in case of preload_app. Also avoids race + # conditions in Rainbows! since load/require are not thread-safe + Unicorn.const_get :HttpResponse end end diff --git a/test/rails/test_rails.rb b/test/rails/test_rails.rb index 304f519..6742722 100644 --- a/test/rails/test_rails.rb +++ b/test/rails/test_rails.rb @@ -234,7 +234,7 @@ logger Logger.new('#{COMMON_TMP.path}') def test_alt_url_root_config_env # cbf to actually work on this since I never use this feature (ewong) return unless ROR_V[0] >= 2 && ROR_V[1] >= 3 - tmp = Tempfile.new(nil) + tmp = Tempfile.new('') tmp.syswrite("ENV['RAILS_RELATIVE_URL_ROOT'] = '/poo'\n") redirect_test_io do @pid = fork { exec 'unicorn_rails', "-l#@addr:#@port", "-c", tmp.path } diff --git a/test/unit/test_http_parser_ng.rb b/test/unit/test_http_parser_ng.rb index 3b9111f..7267ea0 100644 --- a/test/unit/test_http_parser_ng.rb +++ b/test/unit/test_http_parser_ng.rb @@ -172,6 +172,10 @@ class HttpParserNgTest < Test::Unit::TestCase assert_equal '', str assert ! @parser.body_eof? assert_equal "", @parser.filter_body(tmp, "\r\n0\r\n") + assert_equal "", tmp + assert @parser.body_eof? + assert_equal req, @parser.trailers(req, moo = "\r\n") + assert_equal "", moo assert @parser.body_eof? assert ! @parser.keepalive? end diff --git a/test/unit/test_tee_input.rb b/test/unit/test_tee_input.rb index ee81a87..a127882 100644 --- a/test/unit/test_tee_input.rb +++ b/test/unit/test_tee_input.rb @@ -160,7 +160,7 @@ class TestTeeInput < Test::Unit::TestCase pid = fork { @rd.close 5.times { @wr.write("5\r\nabcde\r\n") } - @wr.write("0\r\n") + @wr.write("0\r\n\r\n") } @wr.close ti = Unicorn::TeeInput.new(@rd, @env, @parser, @buf) @@ -199,7 +199,7 @@ class TestTeeInput < Test::Unit::TestCase rd.read(1) == "." and @wr.write("#{'%x' % [ chunk.size]}\r\n#{chunk}\r\n") end - @wr.write("0\r\n") + @wr.write("0\r\n\r\n") } ti = Unicorn::TeeInput.new(@rd, @env, @parser, @buf) assert_nil @parser.content_length @@ -213,6 +213,34 @@ class TestTeeInput < Test::Unit::TestCase assert status.success? end + def test_chunked_with_trailer + @parser = Unicorn::HttpParser.new + @buf = "POST / HTTP/1.1\r\n" \ + "Host: localhost\r\n" \ + "Trailer: Hello\r\n" \ + "Transfer-Encoding: chunked\r\n" \ + "\r\n" + assert_equal @env, @parser.headers(@env, @buf) + assert_equal "", @buf + + pid = fork { + @rd.close + 5.times { @wr.write("5\r\nabcde\r\n") } + @wr.write("0\r\n") + @wr.write("Hello: World\r\n\r\n") + } + @wr.close + ti = Unicorn::TeeInput.new(@rd, @env, @parser, @buf) + assert_nil @parser.content_length + assert_nil ti.len + assert ! @parser.body_eof? + assert_equal 25, ti.size + assert_equal "World", @env['HTTP_HELLO'] + status = nil + assert_nothing_raised { pid, status = Process.waitpid2(pid) } + assert status.success? + end + private def init_parser(body, size = nil) diff --git a/test/unit/test_util.rb b/test/unit/test_util.rb index 4a1e21f..b267179 100644 --- a/test/unit/test_util.rb +++ b/test/unit/test_util.rb @@ -7,7 +7,7 @@ class TestUtil < Test::Unit::TestCase EXPECT_FLAGS = File::WRONLY | File::APPEND def test_reopen_logs_noop - tmp = Tempfile.new(nil) + tmp = Tempfile.new('') tmp.reopen(tmp.path, 'a') tmp.sync = true ext = tmp.external_encoding rescue nil @@ -21,14 +21,14 @@ class TestUtil < Test::Unit::TestCase end def test_reopen_logs_renamed - tmp = Tempfile.new(nil) + tmp = Tempfile.new('') tmp_path = tmp.path.freeze tmp.reopen(tmp_path, 'a') tmp.sync = true ext = tmp.external_encoding rescue nil int = tmp.internal_encoding rescue nil before = tmp.stat.inspect - to = Tempfile.new(nil) + to = Tempfile.new('') File.rename(tmp_path, to.path) assert ! File.exist?(tmp_path) Unicorn::Util.reopen_logs @@ -45,7 +45,7 @@ class TestUtil < Test::Unit::TestCase end def test_reopen_logs_renamed_with_encoding - tmp = Tempfile.new(nil) + tmp = Tempfile.new('') tmp_path = tmp.path.dup.freeze Encoding.list.each { |encoding| File.open(tmp_path, "a:#{encoding.to_s}") { |fp| @@ -68,7 +68,7 @@ class TestUtil < Test::Unit::TestCase end if STDIN.respond_to?(:external_encoding) def test_reopen_logs_renamed_with_internal_encoding - tmp = Tempfile.new(nil) + tmp = Tempfile.new('') tmp_path = tmp.path.dup.freeze Encoding.list.each { |ext| Encoding.list.each { |int| |