From: "kjtsanaktsidis (KJ Tsanaktsidis) via ruby-core" <ruby-core@...>
Date: 2023-11-09T23:41:19+00:00
Subject: [ruby-core:115325] [Ruby master Feature#19995] Proposal: Signal._trap as analogue to Process._fork

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/

 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/