New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Non-overriding signal handlers #2767
Changes from 11 commits
66ba480
73d8e32
f4fda9f
9d157fb
911cb10
06b0d5b
2f3562e
cb0c016
937d46d
7b45e05
2e61866
5383fba
233ac42
4e65f2e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
module Puma | ||
module Signal | ||
module_function | ||
|
||
def prepend_handler(sig, &handler) | ||
name = signame(sig) | ||
custom_signal_handlers[signame] ||= [] | ||
custom_signal_handlers[signame].prepend(handler) | ||
end | ||
nateberkopec marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
# Signal.trap that does not replace | ||
# the existing puma handlers | ||
def trap(sig, handler_str=nil, &handler) | ||
name = signame(sig) | ||
::Signal.trap(name) do | ||
invoke_custom_signal_handlers(name) | ||
|
||
if handler.respond_to?(:call) | ||
yield handler | ||
else | ||
# dirty hack to call string handlers | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, I'm stuck. First of all, I don't like this dirty hack. Besides that, it goes into an infinity loop on the @MSP-Greg could I use your assistance? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @alexeevit I'll have a look a bit later. Interesting failures, may have nothing to do with this PR. Not.sure. Could you rebase? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done 👌 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. Same problem as before... Sorry. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean? You thought the rebase could help? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In ruby 2.5 it just exits and in ruby 2.6 it goes into an infinite loop. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Anyway, I'd like to have a better way to invoke the string handlers for signals (e.g. DEFAULT, SYSTEM_DEFAULT, IGNORE, ...). But this behavior looks quite interesting. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might have the explanation for the problems seen here with Ruby 2.6+, see #3313 (comment) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tested the script in #2767 (comment) with Ruby 3.3, and it exited right away. This seems to suggest that the Note that I tested the script with the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stanhu thanks for the info! |
||
# especially for DEFAULT and SYSTEM_DEFAULT | ||
current_trap = ::Signal.trap(name, handler_str) | ||
Process.kill(name, Process.pid) | ||
::Signal.trap(name, current_trap) | ||
end | ||
end | ||
end | ||
|
||
private | ||
|
||
def signame(sig) | ||
name = sig.is_a?(Integer) ? ::Signal.signame(sig) : String(sig).sub('SIG', '') | ||
raise ArgumentError, "unsupported signal SIG#{name}" unless ::Signal.list[name] | ||
name | ||
end | ||
module_function :signame | ||
|
||
def invoke_custom_signal_handlers(signame) | ||
Array(custom_signal_handlers[signame]).each do |handler| | ||
yield handler | ||
end | ||
end | ||
module_function :invoke_custom_signal_handlers | ||
|
||
def custom_signal_handlers | ||
@custom_signal_handlers ||= {} | ||
end | ||
module_function :custom_signal_handlers | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This exception handling could also be moved to Puma::Signal module
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, I'll consider it if the solution makes sense at all 😄