From 8f5597f7f53f328e1a9e922c521df590b9b8be2f Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sat, 29 Apr 2023 12:05:32 -0700 Subject: [PATCH 1/3] App.Run: Respect Shutdowner ExitCode (#1075) We added support for changing the exit code for a Shutdowner with the ExitCode option in #989, but this was somehow not respected by App.Run. This changes App.Run to use the same underlying machinery (`Wait()`) to decide on the exit code to use. To test this, we add a new internal/e2e submodule that will hold full, end-to-end integration tests. These can be full Fx applications that we run tests against. This is a submodule so that it can have dependencies that are not desirable as direct dependencies of Fx, and it's inside the internal/ directory so that it can consume Fx-internal packages (like testutil). The included regression test verifies the behavior described in #1074. An Fx program using App.Run, and shut down with Shutdowner.Shutdown and an explicit exit code, does not exit with the requested exit code. Failure before the fix: ``` % go test --- FAIL: TestShutdownExitCode (0.01s) writer.go:40: [Fx] PROVIDE fx.Lifecycle <= go.uber.org/fx.New.func1() writer.go:40: [Fx] PROVIDE fx.Shutdowner <= go.uber.org/fx.(*App).shutdowner-fm() writer.go:40: [Fx] PROVIDE fx.DotGraph <= go.uber.org/fx.(*App).dotGraph-fm() writer.go:40: [Fx] INVOKE go.uber.org/fx/internal/e2e/shutdowner_run_exitcode.main.func1() writer.go:40: [Fx] RUNNING writer.go:40: [Fx] TERMINATED main_test.go:46: Error Trace: [..]/fx/internal/e2e/shutdowner_run_exitcode/main_test.go:46 Error: An error is expected but got nil. Test: TestShutdownExitCode FAIL exit status 1 FAIL go.uber.org/fx/internal/e2e/shutdowner_run_exitcode 0.016s ``` Resolves #1074 --- There's a follow up to this: https://github.com/abhinav/fx/pull/1. It depends on the e2e test machinery, so I'll make a PR out of it once this is merged. --- Makefile | 2 +- app.go | 9 +-- app_internal_test.go | 5 +- internal/e2e/README.md | 6 ++ internal/e2e/go.mod | 21 ++++++ internal/e2e/go.sum | 31 ++++++++ internal/e2e/shutdowner_run_exitcode/main.go | 34 +++++++++ .../e2e/shutdowner_run_exitcode/main_test.go | 72 +++++++++++++++++++ 8 files changed, 172 insertions(+), 8 deletions(-) create mode 100644 internal/e2e/README.md create mode 100644 internal/e2e/go.mod create mode 100644 internal/e2e/go.sum create mode 100644 internal/e2e/shutdowner_run_exitcode/main.go create mode 100644 internal/e2e/shutdowner_run_exitcode/main_test.go diff --git a/Makefile b/Makefile index 54953fe94..f03be60ed 100644 --- a/Makefile +++ b/Makefile @@ -9,7 +9,7 @@ GO_FILES = $(shell \ find . '(' -path '*/.*' -o -path './vendor' -o -path '*/testdata/*' ')' -prune \ -o -name '*.go' -print | cut -b3-) -MODULES = . ./tools ./docs +MODULES = . ./tools ./docs ./internal/e2e # 'make cover' should not run on docs by default. # We run that separately explicitly on a specific platform. diff --git a/app.go b/app.go index d2b8caf17..bc3ed09d7 100644 --- a/app.go +++ b/app.go @@ -574,12 +574,12 @@ func (app *App) Run() { // Historically, we do not os.Exit(0) even though most applications // cede control to Fx with they call app.Run. To avoid a breaking // change, never os.Exit for success. - if code := app.run(app.Done()); code != 0 { + if code := app.run(app.Wait()); code != 0 { app.exit(code) } } -func (app *App) run(done <-chan os.Signal) (exitCode int) { +func (app *App) run(done <-chan ShutdownSignal) (exitCode int) { startCtx, cancel := app.clock.WithTimeout(context.Background(), app.StartTimeout()) defer cancel() @@ -588,7 +588,8 @@ func (app *App) run(done <-chan os.Signal) (exitCode int) { } sig := <-done - app.log().LogEvent(&fxevent.Stopping{Signal: sig}) + app.log().LogEvent(&fxevent.Stopping{Signal: sig.Signal}) + exitCode = sig.ExitCode stopCtx, cancel := app.clock.WithTimeout(context.Background(), app.StopTimeout()) defer cancel() @@ -597,7 +598,7 @@ func (app *App) run(done <-chan os.Signal) (exitCode int) { return 1 } - return 0 + return exitCode } // Err returns any error encountered during New's initialization. See the diff --git a/app_internal_test.go b/app_internal_test.go index 6e54f8638..a377dd711 100644 --- a/app_internal_test.go +++ b/app_internal_test.go @@ -23,7 +23,6 @@ package fx import ( "errors" "fmt" - "os" "sync" "testing" @@ -42,7 +41,7 @@ func TestAppRun(t *testing.T) { app := New( WithLogger(func() fxevent.Logger { return spy }), ) - done := make(chan os.Signal) + done := make(chan ShutdownSignal) var wg sync.WaitGroup wg.Add(1) @@ -51,7 +50,7 @@ func TestAppRun(t *testing.T) { app.run(done) }() - done <- _sigINT + done <- ShutdownSignal{Signal: _sigINT} wg.Wait() assert.Equal(t, []string{ diff --git a/internal/e2e/README.md b/internal/e2e/README.md new file mode 100644 index 000000000..0c39606d0 --- /dev/null +++ b/internal/e2e/README.md @@ -0,0 +1,6 @@ +This directory holds end-to-end tests for Fx. +Each subdirectory holds a complete Fx application +and a test for it. + +This is marked as a separate Go module to prevent this code from being bundled +with the Fx library and allow for dependencies that don't leak into Fx. diff --git a/internal/e2e/go.mod b/internal/e2e/go.mod new file mode 100644 index 000000000..c771bc0b7 --- /dev/null +++ b/internal/e2e/go.mod @@ -0,0 +1,21 @@ +module go.uber.org/fx/internal/e2e + +go 1.20 + +require ( + github.com/stretchr/testify v1.8.2 + go.uber.org/fx v1.19.2 +) + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + go.uber.org/atomic v1.7.0 // indirect + go.uber.org/dig v1.16.1 // indirect + go.uber.org/multierr v1.6.0 // indirect + go.uber.org/zap v1.23.0 // indirect + golang.org/x/sys v0.0.0-20220412211240-33da011f77ad // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) + +replace go.uber.org/fx => ../.. diff --git a/internal/e2e/go.sum b/internal/e2e/go.sum new file mode 100644 index 000000000..55ec2f69e --- /dev/null +++ b/internal/e2e/go.sum @@ -0,0 +1,31 @@ +github.com/benbjohnson/clock v1.3.0 h1:ip6w0uFQkncKQ979AypyG0ER7mqUSBdKLOgAle/AT8A= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8= +github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw= +go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= +go.uber.org/dig v1.16.1 h1:+alNIBsl0qfY0j6epRubp/9obgtrObRAc5aD+6jbWY8= +go.uber.org/dig v1.16.1/go.mod h1:557JTAUZT5bUK0SvCwikmLPPtdQhfvLYtO5tJgQSbnk= +go.uber.org/goleak v1.1.11 h1:wy28qYRKZgnJTxGxvye5/wgWr1EKjmUDGYox5mGlRlI= +go.uber.org/multierr v1.6.0 h1:y6IPFStTAIT5Ytl7/XYmHvzXQ7S3g/IeZW9hyZ5thw4= +go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= +go.uber.org/zap v1.23.0 h1:OjGQ5KQDEUawVHxNwQgPpiypGHOxo2mNZsOqTak4fFY= +go.uber.org/zap v1.23.0/go.mod h1:D+nX8jyLsMHMYrln8A0rJjFt/T/9/bGgIhAqxv5URuY= +golang.org/x/sys v0.0.0-20220412211240-33da011f77ad h1:ntjMns5wyP/fN65tdBD4g8J5w8n015+iIIs9rtjXkY0= +golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/e2e/shutdowner_run_exitcode/main.go b/internal/e2e/shutdowner_run_exitcode/main.go new file mode 100644 index 000000000..58872c6ce --- /dev/null +++ b/internal/e2e/shutdowner_run_exitcode/main.go @@ -0,0 +1,34 @@ +// Copyright (c) 2023 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package main + +import ( + "go.uber.org/fx" +) + +func main() { + fx.New( + fx.Invoke(func(shutdowner fx.Shutdowner) error { + shutdowner.Shutdown(fx.ExitCode(20)) + return nil + }), + ).Run() +} diff --git a/internal/e2e/shutdowner_run_exitcode/main_test.go b/internal/e2e/shutdowner_run_exitcode/main_test.go new file mode 100644 index 000000000..849be9fe7 --- /dev/null +++ b/internal/e2e/shutdowner_run_exitcode/main_test.go @@ -0,0 +1,72 @@ +// Copyright (c) 2023 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package main + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/fx/internal/testutil" +) + +// Hijacks the test binary so that the test can run main() as a subprocess +// instead of trying to compile the program and run it directly. +func TestMain(m *testing.M) { + // If the test binary is named "app", then we're running as a subprocess. + // Otherwise, run the tests. + switch filepath.Base(os.Args[0]) { + case "app": + main() + os.Exit(0) + default: + os.Exit(m.Run()) + } +} + +// Verifies that an Fx program running with Run +// exits with the exit code passed to Shutdowner. +// +// Regression test for https://github.com/uber-go/fx/issues/1074. +func TestShutdownExitCode(t *testing.T) { + exe, err := os.Executable() + require.NoError(t, err) + + out := testutil.WriteSyncer{T: t} + + // Run the test binary with the name 'app' so that it runs main(). + cmd := exec.Command(exe) + cmd.Args[0] = "app" + cmd.Stdout = &out + cmd.Stderr = &out + + // The program should exit with code 20. + err = cmd.Run() + require.Error(t, err) + + var exitErr *exec.ExitError + require.ErrorAs(t, err, &exitErr) + + assert.Equal(t, 20, exitErr.ExitCode()) +} From d54a9448ce7392792532e8118baf4838ad8fdc13 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sat, 29 Apr 2023 11:22:42 -0700 Subject: [PATCH 2/3] test: Shutdown from fx.Invoke with App.Start Adds a test for calling Shutdown from an fx.Invoke. This is partially tested in #1075 already, but as reported in #1074, it doesn't work if Start is used. This adds a test for that case as well. --- internal/e2e/shutdowner_wait_exitcode/main.go | 54 ++++++++++++++ .../e2e/shutdowner_wait_exitcode/main_test.go | 72 +++++++++++++++++++ shutdown_test.go | 23 ++++++ 3 files changed, 149 insertions(+) create mode 100644 internal/e2e/shutdowner_wait_exitcode/main.go create mode 100644 internal/e2e/shutdowner_wait_exitcode/main_test.go diff --git a/internal/e2e/shutdowner_wait_exitcode/main.go b/internal/e2e/shutdowner_wait_exitcode/main.go new file mode 100644 index 000000000..f89bfce50 --- /dev/null +++ b/internal/e2e/shutdowner_wait_exitcode/main.go @@ -0,0 +1,54 @@ +// Copyright (c) 2023 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package main + +import ( + "context" + "log" + "os" + "time" + + "go.uber.org/fx" +) + +func main() { + app := fx.New( + fx.Invoke(func(shutdowner fx.Shutdowner) error { + shutdowner.Shutdown(fx.ExitCode(20)) + return nil + }), + ) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + if err := app.Start(ctx); err != nil { + log.Fatal(err) + } + + sig := <-app.Wait() + + if err := app.Stop(ctx); err != nil { + log.Fatal(err) + } + + os.Exit(sig.ExitCode) +} diff --git a/internal/e2e/shutdowner_wait_exitcode/main_test.go b/internal/e2e/shutdowner_wait_exitcode/main_test.go new file mode 100644 index 000000000..849be9fe7 --- /dev/null +++ b/internal/e2e/shutdowner_wait_exitcode/main_test.go @@ -0,0 +1,72 @@ +// Copyright (c) 2023 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package main + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/fx/internal/testutil" +) + +// Hijacks the test binary so that the test can run main() as a subprocess +// instead of trying to compile the program and run it directly. +func TestMain(m *testing.M) { + // If the test binary is named "app", then we're running as a subprocess. + // Otherwise, run the tests. + switch filepath.Base(os.Args[0]) { + case "app": + main() + os.Exit(0) + default: + os.Exit(m.Run()) + } +} + +// Verifies that an Fx program running with Run +// exits with the exit code passed to Shutdowner. +// +// Regression test for https://github.com/uber-go/fx/issues/1074. +func TestShutdownExitCode(t *testing.T) { + exe, err := os.Executable() + require.NoError(t, err) + + out := testutil.WriteSyncer{T: t} + + // Run the test binary with the name 'app' so that it runs main(). + cmd := exec.Command(exe) + cmd.Args[0] = "app" + cmd.Stdout = &out + cmd.Stderr = &out + + // The program should exit with code 20. + err = cmd.Run() + require.Error(t, err) + + var exitErr *exec.ExitError + require.ErrorAs(t, err, &exitErr) + + assert.Equal(t, 20, exitErr.ExitCode()) +} diff --git a/shutdown_test.go b/shutdown_test.go index 1f322a2cb..73f30f50d 100644 --- a/shutdown_test.go +++ b/shutdown_test.go @@ -128,6 +128,29 @@ func TestShutdown(t *testing.T) { assert.NoError(t, s.Shutdown(fx.ExitCode(2), fx.ShutdownTimeout(time.Second))) }) + + t.Run("from invoke", func(t *testing.T) { + t.Parallel() + + app := fxtest.New( + t, + fx.Invoke(func(s fx.Shutdowner) { + s.Shutdown() + }), + ) + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + require.NoError(t, app.Start(ctx), "error starting app") + defer app.Stop(ctx) + select { + case <-ctx.Done(): + assert.Fail(t, "app did not shutdown in time") + case <-app.Wait(): + // success + } + }) } func TestDataRace(t *testing.T) { From fd94f3f946428d4e485ffec03f3a13ea3878e630 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sat, 29 Apr 2023 11:24:06 -0700 Subject: [PATCH 3/3] Support Shutdown from fx.Invoke after App.Start App.Start nils out the "last" signal recorded by signalReceivers, which it otherwise broadcasts to waiters if it was already received. This is unnecessary especially because there's a discrepancy in behavior of using App.Start vs App.Run when shutting down from fx.Invoke. Given a program that calls Shutdown from fx.Invoke, when we do: app := fx.New(...) The shutdowner has already sent the signal, and signalReceivers has already recorded it. At that point, whether we call App.Start or App.Run changes behavior: - If we call App.Run, that calls App.Done (or App.Wait after #1075), which gives it back a channel that already has the signal filled in. It then calls App.Start, waits on the channel--which returns immediately--and then calls App.Stop. - If we call App.Start and App.Wait, on the other hand, Start will clear the signal recorded in signalReceivers, and then App.Wait will build a channel that will block indefinitely because Shutdowner.Shutdown will not be called again. So even though App.Run() and App.Start()+App.Wait() are meant to be equivalent, this causes a serious discrepancy in behavior. It makes sense to resolve this by supporting Shutdown from Invoke. Refs #1074 --- shutdown.go | 2 -- signal.go | 1 - 2 files changed, 3 deletions(-) diff --git a/shutdown.go b/shutdown.go index aa81e68d3..d1e1b9553 100644 --- a/shutdown.go +++ b/shutdown.go @@ -80,8 +80,6 @@ type shutdowner struct { // Shutdown broadcasts a signal to all of the application's Done channels // and begins the Stop process. Applications can be shut down only after they // have finished starting up. -// In practice this means Shutdowner.Shutdown should not be called from an -// fx.Invoke, but from a fx.Lifecycle.OnStart hook. func (s *shutdowner) Shutdown(opts ...ShutdownOption) error { for _, opt := range opts { opt.apply(s) diff --git a/signal.go b/signal.go index 1593c5de2..441ae8a92 100644 --- a/signal.go +++ b/signal.go @@ -109,7 +109,6 @@ func (recv *signalReceivers) Start(ctx context.Context) { return } - recv.last = nil recv.finished = make(chan struct{}, 1) recv.shutdown = make(chan struct{}, 1) recv.notify(recv.signals, os.Interrupt, _sigINT, _sigTERM)