Skip to content

Commit

Permalink
Shutdown: Don't stop receiver, deprecate ShutdownTimeout (#1082)
Browse files Browse the repository at this point in the history
Stacked on top of:

- #1081

However, since I can't push branches directly to this repository,
this PR shows commits from both PRs.

---

Shutdowner.Shutdown is not a blocking operation.
It does not need to block and wait for the relay goroutine to stop;
signalReceiver.Stop will do that.

To clarify this further, signalReceivers has the following operations:

- Start: starts a relay goroutine
- Stop: stops the relay goroutine and waits for it
- Wait: create and return a `chan ShutdownSignal`
- Done: create and return a `chan os.Signal`
- Broadcast: Send the message to all waiting channels

The only reason that the relay goroutine exists is
to map `os.Signal`s received by `os/signal.Notify`
into an `fx.ShutdownSignal`.

Shutdowner.Shutdown should not call signalReceivers.Stop
because the relay goroutine should keep running
so that we can re-use Shutdowner.Shutdown.

This change exposed a leak in TestDataRace (fixed in #1081).

---------

Co-authored-by: Sung Yoon Whang <[email protected]>
  • Loading branch information
abhinav and sywhang authored May 8, 2023
1 parent 808956a commit ec9b096
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 23 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleased
- No changes yet.
### Deprecated
- Deprecate `ShutdownTimeout` option.

## [1.19.1](https://github.com/uber-go/fx/compare/v1.18.0...v1.19.1) - 2023-01-10

Expand Down
25 changes: 5 additions & 20 deletions shutdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
package fx

import (
"context"
"time"
)

Expand Down Expand Up @@ -57,24 +56,23 @@ func ExitCode(code int) ShutdownOption {

type shutdownTimeoutOption time.Duration

func (to shutdownTimeoutOption) apply(s *shutdowner) {
s.shutdownTimeout = time.Duration(to)
}
func (shutdownTimeoutOption) apply(*shutdowner) {}

var _ ShutdownOption = shutdownTimeoutOption(0)

// ShutdownTimeout is a [ShutdownOption] that allows users to specify a timeout
// for a given call to Shutdown method of the [Shutdowner] interface. As the
// Shutdown method will block while waiting for a signal receiver relay
// goroutine to stop.
//
// Deprecated: This option has no effect. Shutdown is not a blocking operation.
func ShutdownTimeout(timeout time.Duration) ShutdownOption {
return shutdownTimeoutOption(timeout)
}

type shutdowner struct {
app *App
exitCode int
shutdownTimeout time.Duration
app *App
exitCode int
}

// Shutdown broadcasts a signal to all of the application's Done channels
Expand All @@ -87,19 +85,6 @@ func (s *shutdowner) Shutdown(opts ...ShutdownOption) error {
opt.apply(s)
}

ctx := context.Background()

if s.shutdownTimeout != time.Duration(0) {
c, cancel := context.WithTimeout(
context.Background(),
s.shutdownTimeout,
)
defer cancel()
ctx = c
}

defer s.app.receivers.Stop(ctx)

return s.app.receivers.Broadcast(ShutdownSignal{
Signal: _sigTERM,
ExitCode: s.exitCode,
Expand Down
4 changes: 2 additions & 2 deletions signal.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func (recv *signalReceivers) Stop(ctx context.Context) error {
}
}

func (recv *signalReceivers) Done() chan os.Signal {
func (recv *signalReceivers) Done() <-chan os.Signal {
recv.m.Lock()
defer recv.m.Unlock()

Expand All @@ -157,7 +157,7 @@ func (recv *signalReceivers) Done() chan os.Signal {
return ch
}

func (recv *signalReceivers) Wait() chan ShutdownSignal {
func (recv *signalReceivers) Wait() <-chan ShutdownSignal {
recv.m.Lock()
defer recv.m.Unlock()

Expand Down

0 comments on commit ec9b096

Please sign in to comment.