(my apologies if?this gets posted twice - don''t think my earlier mail
went through)
Hi,
I''m running unicorn within screen and have run into some trouble.
Unicorn tries to ignore sigwinch when it''s not daemonized.
Unfortunately the check for daemonization fails when unicorn is exec''d
from a process that already has a pgrp.
You can simulate this problem from *within* a screen session with
something like this:
screen sleep 5 && screen bundle exec unicorn -c unicorn.conf
config.ru
sigwinch will be sent to the second screen session the first time you
change to it. This doesn''t happen unless you launch at least two
screens in rapid succession, for reasons that escape me so far -
haven''t dug into that part much yet.
It seems to me that this is a problem, and is occurring because
Unicorn''s check for daemonization is "is init my parent or is my
group
different from my pid?", which isn''t necessarily the same as
checking
whether daemonization has happened.
Here is the patch I would like to see applied:
diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index ae0e175..65880d4 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -282,7 +282,7 @@ class Unicorn::HttpServer
when :USR2 # exec binary, stay alive in case something went wrong
reexec
when :WINCH
- if Process.ppid == 1 || Process.getpgrp != $$
+ if Unicorn::Configurator::RACKUP[:daemonized]
respawn = false
logger.info "gracefully stopping all workers"
kill_each_worker(:QUIT)
Brian P O''Rourke <bpo at somnambulance.net> wrote:> (my apologies if?this gets posted twice - don''t think my earlier mail > went through)I''ve noticed rubyforge has occasional slowness :< It uses Postgrey so first time posters could be delayed...> I''m running unicorn within screen and have run into some trouble. > Unicorn tries to ignore sigwinch when it''s not daemonized. > Unfortunately the check for daemonization fails when unicorn is exec''d > from a process that already has a pgrp. > > You can simulate this problem from *within* a screen session with > something like this: > > screen sleep 5 && screen bundle exec unicorn -c unicorn.conf config.ruSo, start "screen", get a terminal + shell, /then/ type the above?> sigwinch will be sent to the second screen session the first time you > change to it. This doesn''t happen unless you launch at least two > screens in rapid succession, for reasons that escape me so far - > haven''t dug into that part much yet.I haven''t been able to reproduce it on my end. It could be system-dependent (terminals are often wonky/inconsistent).> It seems to me that this is a problem, and is occurring because > Unicorn''s check for daemonization is "is init my parent or is my group > different from my pid?", which isn''t necessarily the same as checking > whether daemonization has happened.Agreed. Regardless of what the problem is, I like your check based on RACKUP[:daemonized]. It''s much more readable and obvious :)> Here is the patch I would like to see applied:I''ll apply it. Is there a commit message you''d like to use? (Otherwise I''ll just edit something based on your email) Thanks!> --- a/lib/unicorn/http_server.rb > +++ b/lib/unicorn/http_server.rb > @@ -282,7 +282,7 @@ class Unicorn::HttpServer > when :USR2 # exec binary, stay alive in case something went wrong > reexec > when :WINCH > - if Process.ppid == 1 || Process.getpgrp != $$ > + if Unicorn::Configurator::RACKUP[:daemonized] > respawn = false > logger.info "gracefully stopping all workers" > kill_each_worker(:QUIT)-- Eric Wong
On Thu, Sep 15, 2011 at 1:32 AM, Eric Wong <normalperson at yhbt.net> wrote:> Brian P O''Rourke <bpo at somnambulance.net> wrote: >> You can simulate this problem from *within* a screen session with >> something like this: >> >> ? ? screen sleep 5 && screen bundle exec unicorn -c unicorn.conf config.ru > > So, start "screen", get a terminal + shell, /then/ type the above? >Correct. When running screen within screen like this, you just get a new screen that executes the command. It happens consistently for me on OSX when I launch several screens at once and the last active screen is *not* unicorn - could certainly be system-dependent.>> Here is the patch I would like to see applied: > > I''ll apply it. ? Is there a commit message you''d like to use? > (Otherwise I''ll just edit something based on your email)You can just pull from my public fork here: git://github.com/bpo/unicorn.git - branch name is ''daemonization_detection'' Cheers, Brian P O''Rourke
Brian P O''Rourke <bpo at somnambulance.net> wrote:> You can just pull from my public fork here: > git://github.com/bpo/unicorn.git - branch name is > ''daemonization_detection''Acked and pushed out as commit b48c6659b294b37f2c6ff3e75c1c9245522d48d1 Thanks!