From 2b067ea07612261398e492a95dc038aeb4070e22 Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Wed, 22 May 2024 12:46:04 +0100
Subject: [PATCH] Use internal channels

As feedback from a code review, use the struct channels as a way
of self documenting the code. This makes the code more readable.
---
 linux_test.go      | 10 ++++------
 lumberjack.go      | 18 +++++++++++-------
 lumberjack_test.go |  5 +++--
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/linux_test.go b/linux_test.go
index a922f74..0063391 100644
--- a/linux_test.go
+++ b/linux_test.go
@@ -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!")
@@ -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.
@@ -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!")
@@ -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.
diff --git a/lumberjack.go b/lumberjack.go
index e137b0d..6224f53 100644
--- a/lumberjack.go
+++ b/lumberjack.go
@@ -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.
@@ -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()
 	}
@@ -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()
 		}()
 	}
diff --git a/lumberjack_test.go b/lumberjack_test.go
index 6a34add..e6382c4 100644
--- a/lumberjack_test.go
+++ b/lumberjack_test.go
@@ -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")
 	}
 }