diff --git a/app.go b/app.go index bc3ed09d7..14e050b6f 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.Wait()); code != 0 { + if code := app.run(app.Wait); code != 0 { app.exit(code) } } -func (app *App) run(done <-chan ShutdownSignal) (exitCode int) { +func (app *App) run(done func() <-chan ShutdownSignal) (exitCode int) { startCtx, cancel := app.clock.WithTimeout(context.Background(), app.StartTimeout()) defer cancel() @@ -587,7 +587,7 @@ func (app *App) run(done <-chan ShutdownSignal) (exitCode int) { return 1 } - sig := <-done + sig := <-done() app.log().LogEvent(&fxevent.Stopping{Signal: sig.Signal}) exitCode = sig.ExitCode diff --git a/app_internal_test.go b/app_internal_test.go index a377dd711..efdc2f33a 100644 --- a/app_internal_test.go +++ b/app_internal_test.go @@ -47,7 +47,7 @@ func TestAppRun(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - app.run(done) + app.run(func() <-chan ShutdownSignal { return done }) }() done <- ShutdownSignal{Signal: _sigINT} 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.go b/shutdown.go index eb67caa87..b5fda5cbd 100644 --- a/shutdown.go +++ b/shutdown.go @@ -78,8 +78,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/shutdown_test.go b/shutdown_test.go index 7565a114e..49956196a 100644 --- a/shutdown_test.go +++ b/shutdown_test.go @@ -128,6 +128,46 @@ 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 + } + }) + + t.Run("many times", func(t *testing.T) { + t.Parallel() + + var shutdowner fx.Shutdowner + app := fxtest.New( + t, + fx.Populate(&shutdowner), + ) + + for i := 0; i < 10; i++ { + app.RequireStart() + shutdowner.Shutdown(fx.ExitCode(i)) + assert.Equal(t, i, (<-app.Wait()).ExitCode, "run %d", i) + app.RequireStop() + } + }) } func TestDataRace(t *testing.T) { diff --git a/signal.go b/signal.go index b804abe7b..1dbfd840e 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) @@ -135,6 +134,7 @@ func (recv *signalReceivers) Stop(ctx context.Context) error { close(recv.finished) recv.shutdown = nil recv.finished = nil + recv.last = nil return nil } }