Skip to content

Commit fa9776f

Browse files
authored
Fix two races (#22187)
* core/cmd: use atomic.Value for shutdown race * core/services/pg: fix log after test race
1 parent 8bb73b5 commit fa9776f

2 files changed

Lines changed: 12 additions & 20 deletions

File tree

core/cmd/shell_local.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"path/filepath"
1313
"strconv"
1414
"strings"
15+
"sync/atomic"
1516
"time"
1617

1718
gethCommon "github.com/ethereum/go-ethereum/common"
@@ -394,18 +395,18 @@ func (s *Shell) runNode(c *cli.Context) error {
394395

395396
// cleanExit is used to skip "fail fast" routine
396397
cleanExit := make(chan struct{})
397-
var shutdownStartTime time.Time
398+
var shutdownStartTime atomic.Value // time.Time
398399
defer func() {
399400
close(cleanExit)
400-
if !shutdownStartTime.IsZero() {
401-
log.Printf("Graceful shutdown time: %s", time.Since(shutdownStartTime))
401+
if val := shutdownStartTime.Load(); val != nil {
402+
log.Printf("Graceful shutdown time: %s", time.Since(val.(time.Time)))
402403
}
403404
}()
404405

405406
go shutdown.HandleShutdown(func(sig string) {
406407
lggr.Infof("Shutting down due to %s signal received...", sig)
407408

408-
shutdownStartTime = time.Now()
409+
shutdownStartTime.Store(time.Now())
409410
cancelRootCtx()
410411

411412
select {

core/services/pg/lease_lock_test.go

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111

1212
"github.com/jmoiron/sqlx"
1313

14-
"github.com/smartcontractkit/chainlink/v2/core/internal/cltest"
1514
"github.com/smartcontractkit/chainlink/v2/core/internal/testutils"
1615
"github.com/smartcontractkit/chainlink/v2/core/logger"
1716
"github.com/smartcontractkit/chainlink/v2/core/services/chainlink"
@@ -177,6 +176,7 @@ func Test_LeaseLock(t *testing.T) {
177176
})
178177

179178
t.Run("cancel TakeAndHold with ctx", func(t *testing.T) {
179+
ctx := t.Context()
180180
cfg := pg.LeaseLockConfig{
181181
DefaultQueryTimeout: cfg.Database().DefaultQueryTimeout(),
182182
LeaseDuration: 15 * time.Second,
@@ -185,23 +185,14 @@ func Test_LeaseLock(t *testing.T) {
185185
leaseLock1 := newLeaseLock(t, db, cfg)
186186
leaseLock2 := newLeaseLock(t, db, cfg)
187187

188-
err := leaseLock1.TakeAndHold(testutils.Context(t))
188+
err := leaseLock1.TakeAndHold(ctx)
189189
require.NoError(t, err)
190+
t.Cleanup(leaseLock1.Release)
190191

191-
awaiter := cltest.NewAwaiter()
192-
go func() {
193-
ctx, cancel := context.WithCancel(testutils.Context(t))
194-
go func() {
195-
<-time.After(3 * time.Second)
196-
cancel()
197-
}()
198-
err := leaseLock2.TakeAndHold(ctx)
199-
require.Error(t, err)
200-
awaiter.ItHappened()
201-
}()
202-
203-
awaiter.AwaitOrFail(t)
204-
leaseLock1.Release()
192+
ctx, cancel := context.WithTimeout(ctx, 3*time.Second)
193+
defer cancel()
194+
require.Error(t, leaseLock2.TakeAndHold(ctx))
195+
t.Cleanup(leaseLock2.Release)
205196
})
206197

207198
require.NoError(t, db.Close())

0 commit comments

Comments
 (0)