Skip to content

Commit e9e8f23

Browse files
author
Ibrahim Jarif
committed
Avoid panic on multiple closer.Signal calls (#1401)
The closer.Signal function closes a channel and multiple calls to the closing the same channel would cause a "channel already closed". This PR avoids the panic by using a sync.Once while closing the channel. Multiple calls to closer.Signal would be no-op with this commit. Fixes DGRAPH-1581
1 parent 4eeba5a commit e9e8f23

2 files changed

Lines changed: 24 additions & 3 deletions

File tree

y/y.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,9 @@ func FixedDuration(d time.Duration) string {
191191
// to tell the goroutine to shut down, and a WaitGroup with which to wait for it to finish shutting
192192
// down.
193193
type Closer struct {
194-
closed chan struct{}
195-
waiting sync.WaitGroup
194+
closed chan struct{}
195+
waiting sync.WaitGroup
196+
closeOnce sync.Once
196197
}
197198

198199
// NewCloser constructs a new Closer, with an initial count on the WaitGroup.
@@ -209,7 +210,10 @@ func (lc *Closer) AddRunning(delta int) {
209210

210211
// Signal signals the HasBeenClosed signal.
211212
func (lc *Closer) Signal() {
212-
close(lc.closed)
213+
// Todo(ibrahim): Change Signal to return error on next badger breaking change.
214+
lc.closeOnce.Do(func() {
215+
close(lc.closed)
216+
})
213217
}
214218

215219
// HasBeenClosed gets signaled when Signal() is called.

y/y_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,3 +254,20 @@ func TestPagebufferReader4(t *testing.T) {
254254
require.Equal(t, err, io.EOF, "should return EOF")
255255
require.Equal(t, n, 0)
256256
}
257+
258+
func TestMulipleSignals(t *testing.T) {
259+
closer := NewCloser(0)
260+
require.NotPanics(t, func() { closer.Signal() })
261+
// Should not panic.
262+
require.NotPanics(t, func() { closer.Signal() })
263+
require.NotPanics(t, func() { closer.SignalAndWait() })
264+
265+
// Attempt 2.
266+
closer = NewCloser(1)
267+
require.NotPanics(t, func() { closer.Done() })
268+
269+
require.NotPanics(t, func() { closer.SignalAndWait() })
270+
// Should not panic.
271+
require.NotPanics(t, func() { closer.SignalAndWait() })
272+
require.NotPanics(t, func() { closer.Signal() })
273+
}

0 commit comments

Comments
 (0)