-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
signal: custom exit code status for interceptor #5636
Changes from all commits
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 | ||
---|---|---|---|---|
|
@@ -75,7 +75,7 @@ func Start(extraArgs string, rpcReady Callback) { | |||
|
||||
// Load the configuration, and parse the extra arguments as command | ||||
// line options. This function will also set up logging properly. | ||||
loadedConfig, err := lnd.LoadConfig(shutdownInterceptor) | ||||
loadedConfig, err := lnd.LoadConfig(*shutdownInterceptor) | ||||
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. Why do we want to dereference here? Can't we just pass a pointer in? 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 initially tried passing it directly to figure out the LoadConfig does not accpet and reference to Interceptor. I was not sure which all aspects of LND would be using the loadConfig as it seems very important to parse the user configs, hence i decided to go the safe way and passed a de-referenced interceptor Line 569 in 9e8b9cc
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. You can bump |
||||
if err != nil { | ||||
atomic.StoreInt32(&lndStarted, 0) | ||||
_, _ = fmt.Fprintln(os.Stderr, err) | ||||
|
@@ -106,7 +106,7 @@ func Start(extraArgs string, rpcReady Callback) { | |||
defer close(quit) | ||||
|
||||
if err := lnd.Main( | ||||
loadedConfig, cfg, shutdownInterceptor, | ||||
loadedConfig, cfg, *shutdownInterceptor, | ||||
); err != nil { | ||||
if e, ok := err.(*flags.Error); ok && | ||||
e.Type == flags.ErrHelp { | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,20 +34,24 @@ type Interceptor struct { | |
|
||
// quit is closed when instructing the main interrupt handler to exit. | ||
quit chan struct{} | ||
|
||
// exitCode is the exit code status when the main interrupt handler exits | ||
exitCode chan int | ||
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. Worth adding a comment here that we must set a value here, otherwise the |
||
} | ||
|
||
// Intercept starts the interception of interrupt signals and returns an `Interceptor` instance. | ||
// Note that any previous active interceptor must be stopped before a new one can be created | ||
func Intercept() (Interceptor, error) { | ||
func Intercept() (*Interceptor, error) { | ||
if !atomic.CompareAndSwapInt32(&started, 0, 1) { | ||
return Interceptor{}, errors.New("intercept already started") | ||
return &Interceptor{}, errors.New("intercept already started") | ||
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. can just return nil here, safe to assume that it won't be used when there's a non-nil error :) |
||
} | ||
|
||
channels := Interceptor{ | ||
interruptChannel: make(chan os.Signal, 1), | ||
shutdownChannel: make(chan struct{}), | ||
shutdownRequestChannel: make(chan struct{}), | ||
quit: make(chan struct{}), | ||
exitCode: make(chan int, 1), | ||
} | ||
|
||
signalsToCatch := []os.Signal{ | ||
|
@@ -59,7 +63,7 @@ func Intercept() (Interceptor, error) { | |
signal.Notify(channels.interruptChannel, signalsToCatch...) | ||
go channels.mainInterruptHandler() | ||
|
||
return channels, nil | ||
return &channels, nil | ||
} | ||
|
||
// mainInterruptHandler listens for SIGINT (Ctrl+C) signals on the | ||
|
@@ -95,16 +99,22 @@ func (c *Interceptor) mainInterruptHandler() { | |
select { | ||
case signal := <-c.interruptChannel: | ||
log.Infof("Received %v", signal) | ||
// Adding exit status code 0 as this will quit after graceful shutdown | ||
c.setExitCode(0) | ||
shutdown() | ||
|
||
case <-c.shutdownRequestChannel: | ||
log.Infof("Received shutdown request.") | ||
// Adding exit status code 0 as this will quit after graceful shutdown | ||
c.setExitCode(1) | ||
DarthBenro008 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
shutdown() | ||
|
||
case <-c.quit: | ||
log.Infof("Gracefully shutting down.") | ||
close(c.shutdownChannel) | ||
signal.Stop(c.interruptChannel) | ||
// Adding exit status code 0 as this will quit after graceful shutdown | ||
c.setExitCode(0) | ||
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. Do we need to set an exit code here? I think we can only hit this case if we've called We can't send multiple values into the channel otherwise we'll end up blocked. I think it's worth adding a comment above |
||
return | ||
} | ||
} | ||
|
@@ -147,3 +157,12 @@ func (c *Interceptor) RequestShutdown() { | |
func (c *Interceptor) ShutdownChannel() <-chan struct{} { | ||
return c.shutdownChannel | ||
} | ||
|
||
// GetExitCode fetches the exit code that has been set of the Interceptor | ||
func (c *Interceptor) GetExitCode() int { | ||
DarthBenro008 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return <-c.exitCode | ||
} | ||
|
||
func (c *Interceptor) setExitCode(exitCode int) { | ||
c.exitCode <- exitCode | ||
} |
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.
still dereferencing here?