Skip to content

Using signals for actions other that shutdown #64

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

Closed
humnandy opened this issue Jul 28, 2020 · 9 comments
Closed

Using signals for actions other that shutdown #64

humnandy opened this issue Jul 28, 2020 · 9 comments
Labels
kind/enhancement Improvements to existing feature.

Comments

@humnandy
Copy link

Is there support for trapping signals for anything other than shutdown hooks?

Linux services will often trap SIGHUP to reload configuration without stopping the service.

I believe that a Windows service can handle a SERVICE_ACCEPT_PARAMCHANGE state request to do the same thing.

This would be a useful feature for production services.

@ktoso
Copy link
Contributor

ktoso commented Jul 28, 2020

This sounds like a reasonable addition to me, I guess you're after similar semantics like httpd or similar, i.e.

Sending the HUP or restart signal to the parent causes it to kill off its children like in TERM, but the parent doesn't exit. It re-reads its configuration files, and re-opens any log files. Then it spawns a new set of children and continues serving hits.

I can see that be useful indeed, esp. if you're managing some child processes...

Have you thought about how to surface it in API?
The current hooks are just a configured var shutdownSignal: [Signal]? though this would be more like an lifecycle.registerSignalHandler(.HUP) { ... } though it should likely not allow registering the ones used for shutdown etc...

WDYT?

Would you like to send in a PR for this?

@ktoso ktoso added the kind/enhancement Improvements to existing feature. label Jul 28, 2020
@ktoso ktoso added this to the 1.0.0-alpha5 milestone Jul 28, 2020
@tomerd
Copy link
Contributor

tomerd commented Aug 10, 2020

+1 on @ktoso design proposal

@tomerd tomerd removed this from the 1.0.0-alpha5 milestone Oct 26, 2020
@hassila
Copy link
Contributor

hassila commented Feb 16, 2021

This would be nice, missed this issue when opening #81

@tomerd
Copy link
Contributor

tomerd commented Mar 11, 2021

@hassila @humnandy ServiceLifecycle exposes a static method trap which can be used to setup signals handlers. Does the help with the need here?

extension ServiceLifecycle {
    /// Setup a signal trap.
    ///
    /// - parameters:
    ///    - signal: The signal to trap.
    ///    - handler: closure to invoke when the signal is captured.
    /// - returns: a `DispatchSourceSignal` for the given trap. The source must be cancelled by the caller.
    public static func trap(signal sig: Signal, handler: @escaping (Signal) -> Void, on queue: DispatchQueue = .global()) -> DispatchSourceSignal {
        let signalSource = DispatchSource.makeSignalSource(signal: sig.rawValue, queue: queue)
        signal(sig.rawValue, SIG_IGN)
        signalSource.setEventHandler(handler: {
            signalSource.cancel()
            handler(sig)
        })
        signalSource.resume()
        return signalSource
    }

    /// A system signal
    public struct Signal: Equatable, CustomStringConvertible {
        internal var rawValue: CInt

        public static let TERM = Signal(rawValue: SIGTERM)
        public static let INT = Signal(rawValue: SIGINT)
        // for testing
        internal static let ALRM = Signal(rawValue: SIGALRM)

        public var description: String {
            var result = "Signal("
            switch self {
            case Signal.TERM: result += "TERM, "
            case Signal.INT: result += "INT, "
            case Signal.ALRM: result += "ALRM, "
            default: () // ok to ignore
            }
            result += "rawValue: \(self.rawValue))"
            return result
        }
    }
}

@hassila
Copy link
Contributor

hassila commented Mar 12, 2021

@tomerd I think that would work perfectly fine, I just did a simple test and it works fine with a small change, see PR: #86
Should probably add SIGHUP too though.

@hassila
Copy link
Contributor

hassila commented Mar 12, 2021

I added support for HUP too.
So for reference if someone stumbles here, a trivial handler for HUP is done thusly:

        let alrmDispatchSourceSignal3 = ServiceLifecycle.trap(signal: ServiceLifecycle.Signal.HUP, handler: { signal in
            print("signal \(signal)")
        }, cancelAfterTrap: false)

(or even better, look at how it's done in setupShutdownHook in Lifecycle.swift...)

@tomerd
Copy link
Contributor

tomerd commented Mar 12, 2021

nice! should we close this issue once your PR lands?

@hassila
Copy link
Contributor

hassila commented Mar 12, 2021

@tomerd definitely, in case any additional signals would be needed in the future they are trivial to add. I think HUP and USR1/USR2 are the most useful ones.

@tomerd
Copy link
Contributor

tomerd commented Mar 14, 2021

closed via #86

@tomerd tomerd closed this as completed Mar 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements to existing feature.
Projects
None yet
Development

No branches or pull requests

4 participants