#123 ✓resolved
Nate Wiger

Thin does not obey QUIT / TERM signal under Ruby 1.9

Reported by Nate Wiger | February 3rd, 2010 @ 09:15 PM | in Future

Related to this earlier ticket: https://thin.lighthouseapp.com/projects/7212/tickets/92-thin-fails-...

We've found in our environment that thin never obeys the QUIT/TERM signal sent from "thin stop". As a result, the server is hard KILL'ed, which is of course dangerous when you have something with a DB connection.

Luckily the solution is a simple 2-line patch to thin/daemonizing.rb. In +daemonize+, just tell thin to listen for those signals explicitly:

  trap('TERM') { logger.info ">> Received TERM signal (PID: #{$$})"; exit }
  trap('QUIT') { logger.info ">> Received QUIT signal (PID: #{$$})"; exit }

Patch attached.

Comments and changes to this ticket

  • macournoyer

    macournoyer February 4th, 2010 @ 08:42 AM

    • State changed from “new” to “resolved”

    (from [99d9ece3feac477871827fc2e41135a79b190814]) Fix Thin not obeying to QUIT / TERM signal under Ruby 1.9 [Nate Wiger] [#123 state:resolved] http://github.com/macournoyer/thin/commit/99d9ece3feac477871827fc2e...

  • macournoyer

    macournoyer February 4th, 2010 @ 08:42 AM

    Thx for the patch Nate!

  • Nate Wiger

    Nate Wiger February 4th, 2010 @ 03:23 PM

    Thanks for the super-quick commit! There's only one thing that's got me confused after re-re-reading the codebase.

    There is this section of code that supposedly does the same thing, in lib/thin/server.rb:

    protected
      # Register signals:
      # * INT calls +stop+ to shutdown gracefully.
      # * TERM calls <tt>stop!</tt> to force shutdown.    
      def setup_signals
        trap('QUIT') { stop }  unless Thin.win?
        trap('INT')  { stop! }
        trap('TERM') { stop! }
      end
    

    But for some reason Ruby 1.9 does no likey.

    So while my patch does indeed work, I'm concerned that it now changes the behavior for "QUIT" from graceful to "forceful" (eg, it bypasses stop which would wait for the backends to shutdown).

    Perhaps this bit of code from Server#initialize doesn't work correctly?

      setup_signals unless options[:signals].class == FalseClass
    

    Bottom line, I'm a little hesitant now about my patch. I'd like to understand "why" it works when Thin is already supposedly doing the same thing. Thoughts?

    Thanks,
    Nate

  • macournoyer

    macournoyer February 11th, 2010 @ 01:54 PM

    • State changed from “resolved” to “open”

    Oops, indeed it was already in there... I'll revert that commit until we figure it out. Thx for reviewing the code.

  • macournoyer

    macournoyer February 11th, 2010 @ 02:57 PM

    Seems the problem is that Ruby 1.9 is not trapping QUIT signal when daemonized. Works fine when not daemonized.

  • Nate Wiger

    Nate Wiger February 12th, 2010 @ 08:50 AM

    Yeah, I was going to ask about that. Both INT and QUIT are designed for terminal control, not daemon control:

    http://en.wikipedia.org/wiki/Signal_(computing)

    Note that QUIT is actually designed to dump core (and INT is Ctrl-C).

    I think the more appropriate signals to use for daemon control would be TERM (rather than INT) for an immediate shutdown, and USR2 as opposed to QUIT for a graceful shutdown. USR1 is usually expected to do something like switch on verbose logging/etc, but I've seen other daemons use USR2 for a graceful restart or shutdown.

    Alternatively, Thin could keep a counter of how many TERM signals it's received. When it receives the first one, it could initiate a soft shutdown via stop() and set "shutting_down = true" or whatever. If a second TERM is received (ie, "shutting_down" is already true) then stop! is immediately called. I've seen this pattern used a lot too. When I was a sysadmin I would always try kill twice then kill -9.

  • Nate Wiger

    Nate Wiger February 12th, 2010 @ 09:41 AM

    Hmmm, looks like Nginx, Unicorn, and Resque all use QUIT in the same fashion as Thin. So regardless of whether it's "right" or not (UNIX-ly speaking), maybe there's code in those daemons that works.

  • macournoyer
  • macournoyer

    macournoyer February 25th, 2010 @ 04:45 PM

    • State changed from “open” to “resolved”

    Now in 1.2.6

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

People watching this ticket

Attachments

Referenced by

Pages