Skip to content

Commit

Permalink
Merge pull request #36 from tilt-dev/nicks/blockuntil
Browse files Browse the repository at this point in the history
fix: fixes a deadlock in BlockUntil
  • Loading branch information
sagikazarmark authored Mar 24, 2022
2 parents f974ef3 + 556fee8 commit 8363671
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 11 deletions.
14 changes: 7 additions & 7 deletions clockwork.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,12 @@ func (fc *fakeClock) After(d time.Duration) <-chan time.Time {
return done
}

// notifyBlockers notifies all the blockers waiting until the
// given number of sleepers are waiting on the fakeClock. It
// returns an updated slice of blockers (i.e. those still waiting)
// notifyBlockers notifies all the blockers waiting until the at least the given
// number of sleepers are waiting on the fakeClock. It returns an updated slice
// of blockers (i.e. those still waiting)
func notifyBlockers(blockers []*blocker, count int) (newBlockers []*blocker) {
for _, b := range blockers {
if b.count == count {
if b.count <= count {
close(b.ch)
} else {
newBlockers = append(newBlockers, b)
Expand Down Expand Up @@ -179,12 +179,12 @@ func (fc *fakeClock) Advance(d time.Duration) {
// (callers of Sleep or After)
func (fc *fakeClock) BlockUntil(n int) {
fc.l.Lock()
// Fast path: current number of sleepers is what we're looking for
if len(fc.sleepers) == n {
// Fast path: we already have >= n sleepers.
if len(fc.sleepers) >= n {
fc.l.Unlock()
return
}
// Otherwise, set up a new blocker
// Otherwise, we have < n sleepers. Set up a new blocker to wait for more.
b := &blocker{
count: n,
ch: make(chan struct{}),
Expand Down
32 changes: 28 additions & 4 deletions clockwork_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,27 @@ func TestNotifyBlockers(t *testing.T) {
b5 := &blocker{10, make(chan struct{})}
bs := []*blocker{b1, b2, b3, b4, b5}
bs1 := notifyBlockers(bs, 2)
if n := len(bs1); n != 4 {
t.Fatalf("got %d blockers, want %d", n, 4)
if n := len(bs1); n != 3 {
t.Fatalf("got %d blockers, want %d", n, 3)
}
select {
case <-b1.ch:
case <-time.After(time.Second):
t.Fatalf("timed out waiting for channel close!")
}
select {
case <-b2.ch:
case <-time.After(time.Second):
t.Fatalf("timed out waiting for channel close!")
}
bs2 := notifyBlockers(bs1, 10)
if n := len(bs2); n != 2 {
t.Fatalf("got %d blockers, want %d", n, 2)
if n := len(bs2); n != 0 {
t.Fatalf("got %d blockers, want %d", n, 0)
}
select {
case <-b3.ch:
case <-time.After(time.Second):
t.Fatalf("timed out waiting for channel close!")
}
select {
case <-b4.ch:
Expand Down Expand Up @@ -144,3 +154,17 @@ func TestFakeClockSince(t *testing.T) {
t.Fatalf("fakeClock.Since() returned unexpected duration, got: %d, want: %d", fc.Since(now), elapsedTime)
}
}

// This used to result in a deadlock.
// https://github.com/jonboulle/clockwork/issues/35
func TestTwoBlockersOneBlock(t *testing.T) {
fc := &fakeClock{}

ft1 := fc.NewTicker(time.Second)
ft2 := fc.NewTicker(time.Second)

fc.BlockUntil(1)
fc.BlockUntil(2)
ft1.Stop()
ft2.Stop()
}

0 comments on commit 8363671

Please sign in to comment.