From b26d3e2c4387707ca958cd9c63c213fc7ac558fa Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Thu, 12 Apr 2012 16:46:24 -0700 Subject: [PATCH] shutdown client socket for apps which fork in background Previously we relied on implicit socket shutdown() from the close() syscall. However, some Rack applications fork() (without calling exec()), creating a potentially long-lived reference to the underlying socket in a child process. This ends up causing nginx to wait on the socket shutdown when the child process exits. Calling shutdown() explicitly signals nginx (or whatever client) that the unicorn worker is done with the socket, regardless of the number of FD references to the underlying socket in existence. This was not an issue for applications which exec() since FD_CLOEXEC is always set on the client socket. Thanks to Patrick Wenger for discovering this. Thanks to Hongli Lai for the tip on using shutdown() as is done in Passenger. ref: http://mid.gmane.org/CAOG6bOTseAPbjU5LYchODqjdF3-Ez4+M8jo-D_D2Wq0jkdc4Rw@mail.gmail.com --- lib/unicorn/http_server.rb | 1 + lib/unicorn/ssl_client.rb | 5 +++++ t/detach.ru | 11 +++++++++++ t/t0021-process_detach.sh | 29 +++++++++++++++++++++++++++++ 4 files changed, 46 insertions(+) create mode 100644 t/detach.ru create mode 100755 t/t0021-process_detach.sh diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb index ede62648..f942e2fe 100644 --- a/lib/unicorn/http_server.rb +++ b/lib/unicorn/http_server.rb @@ -536,6 +536,7 @@ class Unicorn::HttpServer end @request.headers? or headers = nil http_response_write(client, status, headers, body) + client.shutdown # in case of fork() in Rack app client.close # flush and uncork socket immediately, no keepalive rescue => e handle_error(client, e) diff --git a/lib/unicorn/ssl_client.rb b/lib/unicorn/ssl_client.rb index 7b41cd23..a8c79e33 100644 --- a/lib/unicorn/ssl_client.rb +++ b/lib/unicorn/ssl_client.rb @@ -3,4 +3,9 @@ class Unicorn::SSLClient < Kgio::SSL alias write kgio_write alias close kgio_close + + # this is no-op for now, to be fixed in kgio-monkey if people care + # about SSL support... + def shutdown(how = nil) + end end diff --git a/t/detach.ru b/t/detach.ru new file mode 100644 index 00000000..bbd998eb --- /dev/null +++ b/t/detach.ru @@ -0,0 +1,11 @@ +use Rack::ContentType, "text/plain" +fifo_path = ENV["TEST_FIFO"] or abort "TEST_FIFO not set" +run lambda { |env| + pid = fork do + File.open(fifo_path, "wb") do |fp| + fp.write "HIHI" + end + end + Process.detach(pid) + [ 200, {}, [ pid.to_s ] ] +} diff --git a/t/t0021-process_detach.sh b/t/t0021-process_detach.sh new file mode 100755 index 00000000..f03c497e --- /dev/null +++ b/t/t0021-process_detach.sh @@ -0,0 +1,29 @@ +#!/bin/sh +. ./test-lib.sh + +t_plan 5 "Process.detach on forked background process works" + +t_begin "setup and startup" && { + t_fifos process_detach + unicorn_setup + TEST_FIFO=$process_detach \ + unicorn -E none -D detach.ru -c $unicorn_config + unicorn_wait_start +} + +t_begin "read detached PID with HTTP/1.0" && { + detached_pid=$(curl -0 -sSf http://$listen/) + t_info "detached_pid=$detached_pid" +} + +t_begin "read background FIFO" && { + test xHIHI = x"$(cat $process_detach)" +} + +t_begin "killing succeeds" && { + kill $unicorn_pid +} + +t_begin "check stderr" && check_stderr + +t_done -- 2.11.4.GIT