Add support for HUP, USR1, USR2 signals #86

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.

Sign up for GitHub

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

Jump to bottom

Merged

tomerd merged 11 commits into swift-server:main from hassila:hassila-usr-signals

Mar 13, 2021

Contributor

hassila commented Mar 12, 2021

Motivation:
Some servers want to support customised handling of signals except for simply shutting down.

Added support for USR1/USR2 signal traps for (#64) - extended trap() signature with one more optional argument to allow for signal handler to be run multiple times with usage like this:

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

Existing usage of .trap() is unaffected.

Possibly additional signals (HUP?) should be added too.

Changes:
Added USR1/USR2 to Signal struct to allow for trapping them.
Changed .trap() signature to allow for signal handlers to be run multiple times without reregistration.

hassila and others added 4 commits

March 12, 2021 08:24


          Updated target dependency example

a76f7fb

Updated the dependency to use .product as instructed by SPM when trying to follow the instructions.


          Merge pull request #1 from hassila/hassila-readme-patch-1

6ae9879

Updated target dependency example


          Added USR1/USR2 signals

64c7fe4


          Optionally allow to let signal handler run multiple times, useful for…

222475c

… USR signals

swift-server-bot commented Mar 12, 2021

Can one of the admins verify this patch?

6 similar comments

swift-server-bot commented Mar 12, 2021

Can one of the admins verify this patch?

swift-server-bot commented Mar 12, 2021

Can one of the admins verify this patch?

swift-server-bot commented Mar 12, 2021

Can one of the admins verify this patch?

swift-server-bot commented Mar 12, 2021

Can one of the admins verify this patch?

swift-server-bot commented Mar 12, 2021

Can one of the admins verify this patch?

swift-server-bot commented Mar 12, 2021

Can one of the admins verify this patch?

hassila mentioned this pull request

Using signals for actions other that shutdown #64

Closed


          Added HUP signal support.

a33fdc8

Motivation:

Adding support for capturing HUP too in addition to USR1/USR2.

Modifications:

Added HUP signal, added descriptive strings for new signals.

Result:
HUP and USR signals available.

Contributor

ktoso commented Mar 12, 2021

@swift-server-bot test this please

Contributor Author

hassila commented Mar 12, 2021

I added HUP too.

hassila changed the title ~~Add support for USR1/2 signals~~ Add support for HUP, USR1, USR2 signals

Contributor

tomerd commented Mar 12, 2021

thanks @hassila the soundness job reports some formatting issues, see: https://github.com/swift-server/swift-service-lifecycle/blob/main/CONTRIBUTING.md#run-scriptssoundnesssh

tomerd added this to the 1.0.0-alpha.7 milestone

tomerd reviewed

View reviewed changes

Sources/Lifecycle/Lifecycle.swift Outdated

@@ @@ -177,11 +177,13 @@ extension ServiceLifecycle { @@
                   ///    - 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 {
+                  public static func trap(signal sig: Signal, handler: @escaping (Signal) -> Void, on queue: DispatchQueue = .global(), cancelAfterTrap: Bool = true) -> DispatchSourceSignal {

Contributor

tomerd Mar 12, 2021

I prefer we make cancelAfterTrap default to true and change the internal call to set it to false, so the API is nicer to use from a user point of view

Contributor Author

hassila Mar 12, 2021

Definitely agree, +1. Only reason was reflex to not affect possible other users of API. Will fix.

Contributor Author

hassila Mar 12, 2021

I changed default in 8d379dc - missed mentioning it in commit message.

tomerd requested changes

View reviewed changes

Contributor

tomerd left a comment

one comment re. default and formatting issues

hassila added 2 commits

March 12, 2021 21:44


          Trying to fix soundness failures, adding documentation.

8d379dc

Motivation:

Trying to adhere to code standards.

Modifications:

Removing tabs replacing with whitespace.

Result:

Adhering to soundness.sh, adding documentation.


          Remove tab.

921c30c

Contributor Author

hassila commented Mar 12, 2021

@tomerd I tried to run soundness and failed:
jocke@solid /V/j/l/swift-service-lifecycle (hassila-usr-signals)> scripts/soundness.sh
=> Checking for unacceptable language... okay.
=> Checking linux tests... okay.
=> Checking format... ⏎ jocke@solid /V/j/l/swift-service-lifecycle (hassila-usr-signals) [127]>

Assuming the return keys references CRLF issues (just a guess) but nothing obvious (file just edit on macOS and Linux with emacs/xcode). I have removed the tabs that were there previously though.

Committed without tabs and added some documentation.

hassila and others added 2 commits

March 12, 2021 21:59


          Merge branch 'main' into hassila-usr-signals

4dec033


          Whitespace fix.

7fc68b3

Contributor Author

hassila commented Mar 12, 2021

Ok, I have tried to clean whitespace as much as possible, but I get this weird output from soundness.sh which is difficult to guess the purpose of:

jocke@swift511 ~/l/swift-service-lifecycle (hassila-usr-signals)> scripts/soundness.sh
=> Checking for unacceptable language... okay.
=> Checking linux tests... okay.
=> Checking format... ^J                 
jocke@swift511 ~/l/swift-service-lifecycle (hassila-usr-signals) [127]>

Contributor

tomerd commented Mar 12, 2021

@swift-server-bot test this please

Contributor

tomerd commented Mar 12, 2021 •

edited

Loading

Ok, I have tried to clean whitespace as much as possible, but I get this weird output from soundness.sh which is difficult to guess the purpose of:

looks like formatting is still incorrect, run swiftformat . to have it format the project for you

Contributor Author

hassila commented Mar 12, 2021

I tried swift format, but it did changes like:

-    public static func eventLoopFuture(_ future: @escaping () -> EventLoopFuture<Void>) -> LifecycleHandler {
+    static func eventLoopFuture(_ future: @escaping () -> EventLoopFuture<Void>) -> LifecycleHandler {

will look at it bit more, but seemed a bit aggressive there :-)

Contributor

tomerd commented Mar 12, 2021

make sure you use swiftformat not swift-format: brew install swiftformat

Contributor Author

hassila commented Mar 12, 2021

I downloaded and built this one on Linux: https://github.com/nicklockwood/SwiftFormat - not the right one?

Contributor Author

hassila commented Mar 12, 2021

jocke@swift511 ~/l/swift-service-lifecycle (hassila-usr-signals) [1]> swiftformat --version
0.47.12

Contributor

tomerd commented Mar 12, 2021

tool looks fine - I think ci uses 0.44.6

you can see the formatting issue in the log: https://ci.swiftserver.group/job/swift-service-lifecycle-soundness-prb/189/console


          Fix final whitespace (I hope).

1843fb9

Contributor Author

hassila commented Mar 12, 2021

Thanks, I couldn't find that log. Pushed fixes for those now. Seems the new version of swift formatter does a bit more, I will try to fetch 0.44.6 to try to get same output (until CI is updated).

Contributor Author

hassila commented Mar 12, 2021

soundness.sh now passed for formatting at least, thanks for the patience answering questions.

Contributor

tomerd commented Mar 12, 2021

@swift-server-bot test this please

tomerd approved these changes

View reviewed changes

Contributor

tomerd commented Mar 12, 2021

almost there: https://ci.swiftserver.group/job/swift-service-lifecycle-soundness-prb/197/consoleText


          Remove superfluous paranthesis in if statement

265fb15

Contributor Author

hassila commented Mar 13, 2021

Fixed in 265fb15.

Contributor

tomerd commented Mar 13, 2021

@swift-server-bot test this please

Contributor Author

hassila commented Mar 13, 2021

(and installed exact version of swiftformat as used by CI for the future - 0.44.6 doesn't touch as aggressively and can and will now be used... thanks.)

tomerd merged commit ac8ba89 into swift-server:main

Contributor

tomerd commented Mar 13, 2021

thanks @hassila

hassila deleted the hassila-usr-signals branch

March 13, 2021 04:44

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet