Skip to content

Commit 92b568d

Browse files
fix: resource leaks, panics, and improve robustness
- Fix SMTP client leak on reconnection (worker.go) - Fix computeBackoff panic when Attempts=0 (scheduler.go) - Replace ValidateURL HTTP request with url.Parse - Fix break only breaking select not for loop (dispatcher.go) - Fix AttachmentProcessor opening file twice (template.go) - Use os.Stat for --text file detection instead of .txt suffix - Make database path configurable via --db-path flag - Add MustGetGlobalManager for safe singleton access - Stop signal channels on exit to prevent goroutine leak - Fix monitor cleanup waiting arbitrary 5 seconds - Log malformed CSV rows and file close errors - Update tests and docs to match new behavior
1 parent 24900fe commit 92b568d

20 files changed

Lines changed: 323 additions & 371 deletions

File tree

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
BINARY_NAME=mailgrid
66
VERSION ?= $(shell git describe --tags --exact-match 2>/dev/null || git rev-parse --short HEAD)
77
BUILD_TIME=$(shell date -u +%Y-%m-%dT%H:%M:%SZ)
8-
LDFLAGS=-s -w -X main.version=$(VERSION) -X main.buildTime=$(BUILD_TIME)
8+
LDFLAGS=-s -w -X main.version=$(VERSION) -X main.buildTime=$(BUILD_TIME) -X main.commit=$(VERSION)
99

1010
# Optimization flags for minimal binary size
1111
BUILD_FLAGS=-ldflags="$(LDFLAGS)" -trimpath

cli/cliargs.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,9 @@ type CLIArgs struct {
4848
ShowHelp bool // Show help message
4949

5050
// Offset tracking
51-
Resume bool // Resume from last saved offset
52-
ResetOffset bool // Clear offset file and start from beginning
51+
Resume bool // Resume from last saved offset
52+
ResetOffset bool // Clear offset file and start from beginning
53+
DBPath string // Path to the BoltDB database file
5354
}
5455

5556
// printHelp prints a custom formatted help message with grouped flags
@@ -152,6 +153,7 @@ func ParseFlags() CLIArgs {
152153

153154
pflag.BoolVar(&args.Resume, "resume", false, "Resume sending from last saved offset")
154155
pflag.BoolVar(&args.ResetOffset, "reset-offset", false, "Clear offset file and start from beginning")
156+
pflag.StringVar(&args.DBPath, "db-path", "mailgrid.db", "Path to BoltDB database file for job persistence")
155157

156158
// Add help flag manually to control behavior
157159
pflag.BoolVarP(&showHelp, "help", "h", false, "Show this help message")

cli/runner.go

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ func Run(args CLIArgs) error {
4141
// Handle graceful shutdown on Ctrl+C / SIGTERM
4242
sigChan := make(chan os.Signal, 1)
4343
signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM)
44+
defer signal.Stop(sigChan)
4445
var wg sync.WaitGroup
4546
defer wg.Wait()
4647

@@ -55,12 +56,12 @@ func Run(args CLIArgs) error {
5556

5657
// Handle --jobs-list flag
5758
if args.ListJobs {
58-
return listScheduledJobs(args.EnvPath)
59+
return listScheduledJobs(args.DBPath, args.EnvPath)
5960
}
6061

6162
// Handle --jobs-cancel flag
6263
if args.CancelJobID != "" {
63-
return cancelScheduledJob(args.EnvPath, args.CancelJobID)
64+
return cancelScheduledJob(args.DBPath, args.EnvPath, args.CancelJobID)
6465
}
6566

6667
// Run scheduler dispatcher in foreground
@@ -74,7 +75,7 @@ func Run(args CLIArgs) error {
7475
// Configure optimized scheduler manager
7576
config := scheduler.DefaultOptimizedConfig()
7677
managerConfig := scheduler.ManagerConfig{
77-
DBPath: "mailgrid.db",
78+
DBPath: args.DBPath,
7879
SMTPConfig: smtpConfig.SMTP,
7980
OptimizedConfig: config,
8081
ShutdownDelay: 5 * time.Minute,
@@ -178,7 +179,7 @@ func Run(args CLIArgs) error {
178179
}
179180

180181
fmt.Printf("[SCHEDULE] Job scheduled successfully%s\n", scheduleInfo)
181-
fmt.Printf("[DATABASE] Database: mailgrid.db\n")
182+
fmt.Printf("[DATABASE] Database: %s\n", args.DBPath)
182183
fmt.Printf(" The scheduler will start automatically and run in the background\n")
183184

184185
return nil
@@ -411,12 +412,8 @@ func Run(args CLIArgs) error {
411412
// Cleanup monitoring server if it was started
412413
if monitorServer != nil {
413414
go func() {
414-
// Use context with timeout instead of sleep for cleaner shutdown
415-
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
416-
defer cancel()
417-
<-ctx.Done() // Give users time to see final results
418415
if err := monitorServer.Stop(); err != nil {
419-
log.Printf("Failed to stop monitor server: %v", err)
416+
log.Printf("Failed to stop monitor server: %v", err)
420417
}
421418
}()
422419
}
@@ -474,12 +471,12 @@ func Run(args CLIArgs) error {
474471
}
475472

476473
// listScheduledJobs lists all scheduled jobs from the database
477-
func listScheduledJobs(envPath string) error {
474+
func listScheduledJobs(dbPath, envPath string) error {
478475
if envPath == "" {
479476
return fmt.Errorf("config file required (--env)")
480477
}
481478

482-
db, err := database.NewDB("mailgrid.db")
479+
db, err := database.NewDB(dbPath)
483480
if err != nil {
484481
return fmt.Errorf("failed to open database: %w", err)
485482
}
@@ -512,15 +509,15 @@ func listScheduledJobs(envPath string) error {
512509
}
513510

514511
// cancelScheduledJob cancels a scheduled job by ID
515-
func cancelScheduledJob(envPath, jobID string) error {
512+
func cancelScheduledJob(dbPath, envPath, jobID string) error {
516513
if envPath == "" {
517514
return fmt.Errorf("config file required (--env)")
518515
}
519516
if jobID == "" {
520517
return fmt.Errorf("job ID required (--jobs-cancel)")
521518
}
522519

523-
db, err := database.NewDB("mailgrid.db")
520+
db, err := database.NewDB(dbPath)
524521
if err != nil {
525522
return fmt.Errorf("failed to open database: %w", err)
526523
}

cli/tasks.go

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -64,14 +64,11 @@ func PrepareEmailTasks(recipients []parser.Recipient, templatePath, subjectTpl s
6464
return tasks, nil
6565
}
6666

67-
// HasMissingFields returns true if any field in recipient data is empty.
67+
// HasMissingFields returns true if the recipient email is empty.
68+
// Other data fields may be optional depending on the template, so only
69+
// the email field is required.
6870
func HasMissingFields(r parser.Recipient) bool {
69-
for _, val := range r.Data {
70-
if val == "" {
71-
return true
72-
}
73-
}
74-
return false
71+
return r.Email == ""
7572
}
7673

7774
// printDryRun logs rendered email content to the console instead of sending.
@@ -179,14 +176,11 @@ func SendSingleEmail(args CLIArgs, cfg config.SMTPConfig) error {
179176

180177
fmt.Printf(" Monitor dashboard: http://localhost:%d\n", args.MonitorPort)
181178

182-
// Cleanup monitor after completion with context timeout instead of sleep
179+
// Cleanup monitor after completion
183180
defer func() {
184181
go func() {
185-
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
186-
defer cancel()
187-
<-ctx.Done() // Wait for timeout or cancellation
188182
if err := monitorServer.Stop(); err != nil {
189-
log.Printf("Failed to stop monitor server: %v", err)
183+
log.Printf("Failed to stop monitor server: %v", err)
190184
}
191185
}()
192186
}()

cmd/mailgrid/main.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"fmt"
66
"github.com/bravo1goingdark/mailgrid/cli"
77
"log"
8-
"os"
98
)
109

1110
// Version information (set at build time)
@@ -45,5 +44,4 @@ func showVersion() {
4544
fmt.Printf("Commit: %s\n", commit)
4645
fmt.Printf("\nMailGrid is a production-ready email orchestrator for bulk email campaigns.\n")
4746
fmt.Printf("Documentation: https://github.com/bravo1goingdark/mailgrid\n")
48-
os.Exit(0)
4947
}

config/config.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ func LoadConfig(path string) (*AppConfig, error) {
3131
}
3232
defer func() {
3333
if closeErr := file.Close(); closeErr != nil {
34-
// Log error but don't override main function error
35-
// This ensures we don't mask the main error if JSON decoding fails
34+
// Log to stderr since callers may override the returned error
35+
fmt.Fprintf(os.Stderr, "Warning: failed to close config file: %v\n", closeErr)
3636
}
3737
}()
3838

docs/docs.md

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,14 +104,15 @@ Subject line with template support.
104104

105105
### `--text`
106106

107-
Plain text body (inline or `.txt` file).
107+
Plain text body (inline text or path to a file). If the argument points to an existing file, its contents are used; otherwise it's treated as inline text.
108108

109109
```bash
110110
# Inline
111111
--text "Hello world!"
112112

113-
# From file
113+
# From file (any extension)
114114
--text ./body.txt
115+
--text /path/to/message
115116
```
116117

117118
### `--to`
@@ -124,10 +125,13 @@ mailgrid --env config.json --to user@example.com --subject "Hello" --text "Hi!"
124125

125126
### `--cc` / `--bcc`
126127

127-
CC/BCC recipients (comma-separated or `@file.txt`).
128+
CC/BCC recipients (comma-separated emails or path to a file with one email per line).
128129

129130
```bash
131+
# Inline
130132
--cc "team@example.com,manager@example.com"
133+
134+
# From file
131135
--bcc ./bcc_list.txt
132136
```
133137

@@ -240,6 +244,8 @@ Schedule emails for later or recurring delivery. Jobs persist in BoltDB.
240244
```bash
241245
--job-retries 3 # Scheduler-level retries (default: 3)
242246
-J 3
247+
248+
--db-path mailgrid.db # Path to BoltDB database (default: mailgrid.db)
243249
```
244250

245251
---
@@ -411,3 +417,4 @@ mailgrid --env config.json \
411417
| `--scheduler-run` | `-R` | false | Run daemon |
412418
| `--resume` | - | false | Resume campaign |
413419
| `--reset-offset` | - | false | Clear offset |
420+
| `--db-path` | - | mailgrid.db | Database path |

email/dispatcher.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func StartDispatcher(tasks []Task, cfg config.SMTPConfig, concurrency int, batch
130130
case taskChan <- task:
131131
case <-ctx.Done():
132132
log.Printf("Context cancelled, stopping task dispatch")
133-
break
133+
return // return instead of break to exit the goroutine
134134
}
135135
}
136136
close(taskChan)

email/template.go

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"crypto/sha256"
66
"encoding/hex"
77
"errors"
8+
"fmt"
89
"html/template"
910
"io"
1011
"mime"
@@ -227,7 +228,8 @@ func NewAttachmentProcessor(maxSize int64) *AttachmentProcessor {
227228
}
228229
}
229230

230-
// ProcessAttachment handles a single attachment efficiently
231+
// ProcessAttachment handles a single attachment efficiently.
232+
// It opens the file only once and reuses the handle for both MIME detection and reading.
231233
func (p *AttachmentProcessor) ProcessAttachment(path string) (io.Reader, string, error) {
232234
// Get file info
233235
info, err := os.Stat(path)
@@ -240,35 +242,36 @@ func (p *AttachmentProcessor) ProcessAttachment(path string) (io.Reader, string,
240242
return nil, "", ErrAttachmentTooLarge
241243
}
242244

243-
// Check MIME type
245+
// Open file once
246+
file, err := os.Open(path)
247+
if err != nil {
248+
return nil, "", err
249+
}
250+
251+
// Check MIME type by extension first
244252
mimeType := mime.TypeByExtension(filepath.Ext(path))
245253
if mimeType == "" {
246-
// Try to detect type
247-
file, err := os.Open(path)
248-
if err != nil {
249-
return nil, "", err
254+
// Try to detect type from file content
255+
buffer := make([]byte, 512)
256+
n, readErr := file.Read(buffer)
257+
if readErr != nil && readErr != io.EOF {
258+
file.Close()
259+
return nil, "", readErr
250260
}
251-
defer file.Close()
261+
mimeType = http.DetectContentType(buffer[:n])
252262

253-
// Read first 512 bytes for MIME detection
254-
buffer := make([]byte, 512)
255-
_, err = file.Read(buffer)
256-
if err != nil && err != io.EOF {
257-
return nil, "", err
263+
// Seek back to beginning for actual reading
264+
if _, seekErr := file.Seek(0, io.SeekStart); seekErr != nil {
265+
file.Close()
266+
return nil, "", fmt.Errorf("failed to seek file: %w", seekErr)
258267
}
259-
mimeType = http.DetectContentType(buffer)
260268
}
261269

262270
if _, ok := p.allowedTypes[mimeType]; !ok {
271+
file.Close()
263272
return nil, "", ErrUnsupportedAttachmentType
264273
}
265274

266-
// Create efficient reader
267-
file, err := os.Open(path)
268-
if err != nil {
269-
return nil, "", err
270-
}
271-
272275
// Use buffered reader from pool
273276
buf := p.bufferPool.Get().(*[]byte)
274277
reader := bufio.NewReaderSize(file, len(*buf))

email/worker.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,10 @@ func processBatch(w worker, client *smtp.Client, batch []Task) {
185185
// Check if we need to reconnect (connection lost or auth error)
186186
if isConnectionError(err) {
187187
log.Printf("[Worker %d] Connection error, attempting reconnection...", w.ID)
188+
// Close old client before creating a new one to prevent resource leak
189+
if quitErr := client.Quit(); quitErr != nil {
190+
log.Printf("[Worker %d] Failed to quit old SMTP session: %v", w.ID, quitErr)
191+
}
188192
newClient, reconnErr := ConnectSMTPWithContext(w.Ctx, w.Config)
189193
if reconnErr != nil {
190194
log.Printf("[Worker %d] Reconnection failed: %v", w.ID, reconnErr)

0 commit comments

Comments
 (0)