[ruby-core:115325] [Ruby master Feature#19995] Proposal: Signal._trap as analogue to Process._fork

9 Nov 2023

      Issue #19995 has been reported by kjtsanaktsidis (KJ Tsanaktsidis).

----------------------------------------
Feature #19995: Proposal: Signal._trap as analogue to Process._fork
https://bugs.ruby-lang.org/issues/19995

* Author: kjtsanaktsidis (KJ Tsanaktsidis)
* Status: Open
* Priority: Normal
----------------------------------------
This is a proposal to define a method `Signal#_trap`, which would function as a single place to be wrapped by monitoring libraries for detecting when signal handlers are entered and exited.

## Motivation #1 - stack-based context leaks into signal handlers

When writing monitoring & instrumentation libraries, we often want to add context to emitted logs/metrics/etc based on the current call stack. For example, `ActiveSupport::TaggedLogging` allows you to write code like this:

```
$logger = ActiveSupport::TaggedLogging.new(Logger.new($stderr))

def method_one
    $logger.tagged("method_one") do
        method_two
    end
end

def method_two
    $logger.tagged("method_two") do
        $logger.info "hello there"
    end
end

method_one # logs "[method_one] [method_two] hello there"
```

Signal handlers can complicate this picture though because they can run at arbitrary points in the program. Using a threadlocal variable to store context, as is commonly done, means that the invoked signal handler shares the context of whatever thread it interrupted. For example, this program will log the "method_one" tag from inside the signal handler:

```
# n.b. - need to use the mono_logger gem instead of stdlib ::Logger because ::Logger contains a mutex,
# which cannot be used from a trap handler.
$logger = ActiveSupport::TaggedLogging.new(MonoLogger.new($stderr))
Signal.trap(:TERM) do
    $logger.tagged("term_handler") do
        $logger.info "goodbye there"
        exit
    end
end

def method_one
    $logger.tagged("method_one") do
        Process.kill :TERM, Process.pid
        sleep
    end
end

method_one # logs "[method_one] [term_handler] goodbye there"
```

This is, in my opinion, undesirable. The fact that the signal handler happened to interrupt at this point and not some other point does not mean that the handler has anything at all to do with `method_one`. I would like a way to, in the logger implementation, detect that we have entered a signal handler and switch to a different context stack, so that the above example printed "[term_handler] goodbye there" instead.

## Motivation #2 - avoiding mutex use in signal handlers

This motivation is hinted at by the use of the `mono_logger` gem in the preceding example. It’s not legal to use a mutex in a trap handler, which means `::Logger` can’t be used. However, the mutex is there for a good reason - it synchronises writes to the log device amongst multiple threads so that messages are written atomically, even if they’re longer than `PIPE_BUF` (the max size write the OS guarantees is atomic in a single write syscall).  Ideally, we would like to use the mutex-based implementation in normal code, but avoid the mutex in trap handlers (possibly writing to a different stream e.g. `$stderr` instead of `$stdout`). 

For an instrumentation implementation to do this, it needs a way to detect if it’s currently running inside a trap handler.

## Proposed solution

I would like to add a "hook" method `Signal._trap`, which works in a similar way to `Process._fork`. Essentially, when Ruby invokes a signal handler, instead of directly invoking the registered proc, it would invoke `Signal._trap`, and _that_ would invoke the proc. This means that by prepending to `Signal.singleton_class`, you could write code which wraps around all signal handlers. My first example could then be re-written like so:

```
$logger = ActiveSupport::TaggedLogging.new(MonoLogger.new($stderr))

Signal.singleton_class.prepend(Module.new do
    def _trap(signo)
        old_tags = $logger.pop_tags 1_000_000_000 # ActiveSupport hack; there's no "pop_all" method.
        super
        $logger.push_tags old_tags
    end
end)

Signal.trap(:TERM) do
    $logger.tagged("term_handler") do
        $logger.info "goodbye there"
        exit
    end
end

def method_one
    $logger.tagged("method_one") do
        Process.kill :TERM, Process.pid
        sleep
    end
end

method_one # logs "[term_handler] goodbye there"
```

## Polyfill for older Rubies

I believe `Signal._trap` can be fully implemented in a gem for older versions of Ruby; something like the following will work: https://gist.github.com/KJTsanaktsidis/0b263c76523a16a049fa5a035e868a68. This is, again, analogous to how monitoring libraries would handle fork hooking before `_fork` was added - possible, but very tricky and ugly.

## Alternative solution

Another option which might help solve the two problems I outlined is to expose the `ec->interrupt_mask |= TRAP_INTERRUPT_MASK flag` as a method on `Signal` or a special `$MAGIC_GLOBAL` variable. This would essentially let code detect whether or not it’s in a trap handler, but not execute specific code around trap handlers.

----

Thanks for your time friends, looking forward to any feedback or discussion!

-- 
https://bugs.ruby-lang.org/