Skip to content

Commit 6c39f9e

Browse files
committed
Fix: Close log file in Builder.Close() to prevent resource leak
Added proper cleanup of the log file opened in buildLogger() to prevent file descriptor leaks in long-running deployments. The Close() method now properly closes both the log file and RabbitMQ connections. Changes: - Added nil-safe log file closure in Builder.Close() method - Added comprehensive unit tests to verify log file is properly closed - Tests cover normal case, nil log file, and builder without log file
1 parent e168bb4 commit 6c39f9e

File tree

2 files changed

+73
-0
lines changed

2 files changed

+73
-0
lines changed

builder/builder.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,14 @@ func (b *Builder) buildRabbitMQ(rabbitMQURI string) error {
7878
}
7979

8080
func (b *Builder) Close() error {
81+
// Close log file if it was opened
82+
if b.logsFile != nil {
83+
if err := b.logsFile.Close(); err != nil {
84+
return err
85+
}
86+
}
87+
88+
// Close RabbitMQ connections
8189
if b.rabbitMQConnection != nil {
8290
if err := b.rabbitMQChannel.Close(); err != nil {
8391
return err

builder/builder_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package builder
2+
3+
import (
4+
"os"
5+
"testing"
6+
7+
"github.com/mariocandela/beelzebub/v3/parser"
8+
"github.com/stretchr/testify/assert"
9+
)
10+
11+
func TestBuilderClose_LogFile(t *testing.T) {
12+
// Create a temporary directory for the test
13+
tmpDir := t.TempDir()
14+
logFilePath := tmpDir + "/test.log"
15+
16+
// Create a builder instance
17+
builder := NewBuilder()
18+
19+
// Build logger which opens a log file
20+
loggingConfig := parser.Logging{
21+
Debug: false,
22+
DebugReportCaller: false,
23+
LogDisableTimestamp: true,
24+
LogsPath: logFilePath,
25+
}
26+
27+
err := builder.buildLogger(loggingConfig)
28+
assert.NoError(t, err)
29+
assert.NotNil(t, builder.logsFile)
30+
31+
// Verify the log file exists and is open
32+
fileInfo, err := os.Stat(logFilePath)
33+
assert.NoError(t, err)
34+
assert.NotNil(t, fileInfo)
35+
36+
// Close the builder
37+
err = builder.Close()
38+
assert.NoError(t, err)
39+
40+
// Verify the log file is closed by attempting to write to it
41+
// Writing to a closed file should return an error
42+
_, err = builder.logsFile.WriteString("test")
43+
assert.Error(t, err)
44+
assert.Contains(t, err.Error(), "file already closed")
45+
}
46+
47+
func TestBuilderClose_NoLogFile(t *testing.T) {
48+
// Create a builder without opening a log file
49+
builder := NewBuilder()
50+
51+
// Close should succeed even without a log file
52+
err := builder.Close()
53+
assert.NoError(t, err)
54+
}
55+
56+
func TestBuilderClose_NilLogFile(t *testing.T) {
57+
// Create a builder with explicitly nil log file
58+
builder := &Builder{
59+
logsFile: nil,
60+
}
61+
62+
// Close should succeed with nil log file
63+
err := builder.Close()
64+
assert.NoError(t, err)
65+
}

0 commit comments

Comments
 (0)