Skip to content

Commit

Permalink
Use internal channels
Browse files Browse the repository at this point in the history
As feedback from a code review, use the struct channels as a way
of self documenting the code. This makes the code more readable.
  • Loading branch information
SimonRichardson committed May 22, 2024
1 parent c81c014 commit 2b067ea
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 15 deletions.
10 changes: 4 additions & 6 deletions linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,12 @@ func TestCompressMaintainMode(t *testing.T) {
isNil(err, t)
f.Close()

notify := make(chan struct{})
l := &Logger{
Compress: true,
Filename: filename,
MaxBackups: 1,
MaxSize: 100, // megabytes
notifyCompressed: notify,
notifyCompressed: make(chan struct{}),
}
defer l.Close()
b := []byte("boo!")
Expand All @@ -116,7 +115,7 @@ func TestCompressMaintainMode(t *testing.T) {
err = l.Rotate()
isNil(err, t)

waitForNotify(notify, t)
waitForNotify(l.notifyCompressed, t)

// a compressed version of the log file should now exist with the correct
// mode.
Expand Down Expand Up @@ -147,13 +146,12 @@ func TestCompressMaintainOwner(t *testing.T) {
isNil(err, t)
f.Close()

notify := make(chan struct{})
l := &Logger{
Compress: true,
Filename: filename,
MaxBackups: 1,
MaxSize: 100, // megabytes
notifyCompressed: notify,
notifyCompressed: make(chan struct{}),
}
defer l.Close()
b := []byte("boo!")
Expand All @@ -166,7 +164,7 @@ func TestCompressMaintainOwner(t *testing.T) {
err = l.Rotate()
isNil(err, t)

waitForNotify(notify, t)
waitForNotify(l.notifyCompressed, t)

// a compressed version of the log file should now exist with the correct
// owner.
Expand Down
18 changes: 11 additions & 7 deletions lumberjack.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,16 +175,20 @@ func (l *Logger) Write(p []byte) (n int, err error) {
func (l *Logger) Close() error {
l.mu.Lock()
defer l.mu.Unlock()
if err := l.close(); err != nil {
return err
}

// Always close the mill channel, even if the close fails. This way we
// guarantee that the mill goroutine will exit.
err := l.close()

if l.millCh != nil {
close(l.millCh)
l.wg.Wait()
l.millCh = nil
l.wg = nil
}
return nil

// Return the result of the file close.
return err
}

// close closes the file if it is open.
Expand Down Expand Up @@ -404,8 +408,8 @@ func (l *Logger) millRunOnce() error {

// millRun runs in a goroutine to manage post-rotation compression and removal
// of old log files.
func (l *Logger) millRun(ch <-chan struct{}) {
for range ch {
func (l *Logger) millRun() {
for range l.millCh {
// what am I going to do, log this?
_ = l.millRunOnce()
}
Expand All @@ -420,7 +424,7 @@ func (l *Logger) mill() {
l.wg = &sync.WaitGroup{}
l.wg.Add(1)
go func() {
l.millRun(l.millCh)
l.millRun()
l.wg.Done()
}()
}
Expand Down
5 changes: 3 additions & 2 deletions lumberjack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -817,11 +817,12 @@ func exists(path string, t testing.TB) {
}

func waitForNotify(notify <-chan struct{}, t testing.TB) {
t.Helper()

select {
case <-notify:
// All good.
case <-time.After(2 * time.Second):
fmt.Println("logger notify not signalled")
t.FailNow()
t.Fatal("logger notify not signalled")
}
}

0 comments on commit 2b067ea

Please sign in to comment.