Skip to content

Commit

Permalink
fix: handle context cancellations during instance refresh (#372)
Browse files Browse the repository at this point in the history
Fixes #370
  • Loading branch information
jault3 committed Nov 9, 2022
1 parent 674ccb3 commit cdb59c7
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 6 deletions.
15 changes: 9 additions & 6 deletions internal/cloudsql/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,17 @@ func (i *Instance) scheduleRefresh(d time.Duration) *refreshOperation {
res.md, res.tlsCfg, res.expiry, res.err = i.r.performRefresh(i.ctx, i.connName, i.key, i.RefreshCfg.UseIAMAuthN)
close(res.ready)

select {
case <-i.ctx.Done():
// instance has been closed, don't schedule anything
return
default:
}

// Once the refresh is complete, update "current" with working result and schedule a new refresh
i.resultGuard.Lock()
defer i.resultGuard.Unlock()

// if failed, scheduled the next refresh immediately
if res.err != nil {
i.next = i.scheduleRefresh(0)
Expand All @@ -308,14 +316,9 @@ func (i *Instance) scheduleRefresh(d time.Duration) *refreshOperation {
}
return
}

// Update the current results, and schedule the next refresh in the future
i.cur = res
select {
case <-i.ctx.Done():
// instance has been closed, don't schedule anything
return
default:
}
t := refreshDuration(time.Now(), i.cur.expiry)
i.next = i.scheduleRefresh(t)
})
Expand Down
29 changes: 29 additions & 0 deletions internal/cloudsql/instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,3 +303,32 @@ func TestRefreshDuration(t *testing.T) {
})
}
}

func TestContextCancelled(t *testing.T) {
ctx := context.Background()

client, cleanup, err := mock.NewSQLAdminService(ctx)
if err != nil {
t.Fatalf("%s", err)
}
defer cleanup()

// Set up an instance and then close it immediately
im, err := NewInstance("my-proj:my-region:my-inst", client, RSAKey, 30, nil, "", RefreshCfg{})
if err != nil {
t.Fatalf("failed to initialize Instance: %v", err)
}
im.Close()

// grab the current value of next before scheduling another refresh
next := im.next

op := im.scheduleRefresh(time.Nanosecond)
<-op.ready

// if scheduleRefresh returns without scheduling another one,
// i.next should be untouched and remain the same pointer value
if im.next != next {
t.Fatalf("refresh did not return after a closed context. next pointer changed: want = %p, got = %p", next, im.next)
}
}

0 comments on commit cdb59c7

Please sign in to comment.