Skip to content

Commit

Permalink
Revert "Ensure OnStart/OnStop hooks can only be called once (#931)" (#…
Browse files Browse the repository at this point in the history
…946)

This reverts commit 5566339.
  • Loading branch information
sywhang authored Sep 28, 2022
1 parent 9603fe9 commit 550fe46
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 74 deletions.
52 changes: 20 additions & 32 deletions app.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,6 @@ type App struct {
dones []chan os.Signal
shutdownSig os.Signal

// Used to make sure Start/Stop is called only once.
runStart sync.Once
runStop sync.Once

osExit func(code int) // os.Exit override; used for testing only
}

Expand Down Expand Up @@ -662,25 +658,21 @@ var (
// Note that Start short-circuits immediately if the New constructor
// encountered any errors in application initialization.
func (app *App) Start(ctx context.Context) (err error) {
app.runStart.Do(func() {
defer func() {
app.log.LogEvent(&fxevent.Started{Err: err})
}()
defer func() {
app.log.LogEvent(&fxevent.Started{Err: err})
}()

if app.err != nil {
// Some provides failed, short-circuit immediately.
err = app.err
return
}
if app.err != nil {
// Some provides failed, short-circuit immediately.
return app.err
}

err = withTimeout(ctx, &withTimeoutParams{
hook: _onStartHook,
callback: app.start,
lifecycle: app.lifecycle,
log: app.log,
})
return withTimeout(ctx, &withTimeoutParams{
hook: _onStartHook,
callback: app.start,
lifecycle: app.lifecycle,
log: app.log,
})
return
}

func (app *App) start(ctx context.Context) error {
Expand Down Expand Up @@ -708,20 +700,16 @@ func (app *App) start(ctx context.Context) error {
// called are executed. However, all those hooks are executed, even if some
// fail.
func (app *App) Stop(ctx context.Context) (err error) {
app.runStop.Do(func() {
// Protect the Stop hooks from being called multiple times.
defer func() {
app.log.LogEvent(&fxevent.Stopped{Err: err})
}()
defer func() {
app.log.LogEvent(&fxevent.Stopped{Err: err})
}()

err = withTimeout(ctx, &withTimeoutParams{
hook: _onStopHook,
callback: app.lifecycle.Stop,
lifecycle: app.lifecycle,
log: app.log,
})
return withTimeout(ctx, &withTimeoutParams{
hook: _onStopHook,
callback: app.lifecycle.Stop,
lifecycle: app.lifecycle,
log: app.log,
})
return
}

// Done returns a channel of signals to block on after starting the
Expand Down
42 changes: 0 additions & 42 deletions app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
"reflect"
"runtime"
"strings"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -1282,47 +1281,6 @@ func TestAppStart(t *testing.T) {
err := app.Start(context.Background()).Error()
assert.Contains(t, err, "OnStart hook added by go.uber.org/fx_test.TestAppStart.func10.1 failed: goroutine exited without returning")
})

t.Run("Start/Stop should be called exactly once only.", func(t *testing.T) {
t.Parallel()
startCalled := 0
stopCalled := 0
app := fxtest.New(t,
Provide(Annotate(func() int { return 0 },
OnStart(func(context.Context) error {
startCalled += 1
return nil
}),
OnStop(func(context.Context) error {
stopCalled += 1
return nil
})),
),
Invoke(func(i int) {
assert.Equal(t, 0, i)
}),
)
var wg sync.WaitGroup
for i := 0; i < 10; i++ {
wg.Add(1)
go func() {
defer wg.Done()
app.Start(context.Background())
}()
}
wg.Wait()
assert.Equal(t, 1, startCalled)
for i := 0; i < 10; i++ {
wg.Add(1)
go func() {
defer wg.Done()
app.Stop(context.Background())
}()
}
wg.Wait()
assert.Equal(t, 1, stopCalled)
})

}

func TestAppStop(t *testing.T) {
Expand Down

0 comments on commit 550fe46

Please sign in to comment.