Skip to content

Commit 62ee1a4

Browse files
CopilotZozman
andcommitted
Implement comprehensive memory leak fixes and resource management
Co-authored-by: Zozman <1192364+Zozman@users.noreply.github.com>
1 parent 8b4ecce commit 62ee1a4

5 files changed

Lines changed: 192 additions & 7 deletions

File tree

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
# Output of the go coverage tool, specifically when used with LiteIDE
1212
*.out
1313

14+
# Build artifacts
15+
stream
16+
1417
# Dependency directories (remove the comment below to include it)
1518
# vendor/
1619

cmd/main.go

Lines changed: 133 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"os"
99
"os/exec"
1010
"os/signal"
11+
"runtime"
1112
"strconv"
1213
"strings"
1314
"sync"
@@ -82,18 +83,52 @@ func (s *StreamState) stopStream(logger *zap.Logger) {
8283

8384
logger.Info("Stopping existing stream...")
8485

85-
// Stop FFmpeg process
86+
// Stop FFmpeg process with proper cleanup
8687
if s.ffmpegCmd != nil && s.ffmpegCmd.Process != nil {
8788
logger.Debug("Terminating FFmpeg process")
88-
if err := s.ffmpegCmd.Process.Kill(); err != nil {
89-
logger.Warn("Failed to kill FFmpeg process", zap.Error(err))
89+
90+
// First try graceful termination with SIGTERM
91+
if err := s.ffmpegCmd.Process.Signal(syscall.SIGTERM); err != nil {
92+
logger.Warn("Failed to send SIGTERM to FFmpeg process", zap.Error(err))
93+
} else {
94+
// Give process 3 seconds to terminate gracefully
95+
done := make(chan error, 1)
96+
go func() {
97+
done <- s.ffmpegCmd.Wait()
98+
}()
99+
100+
select {
101+
case <-done:
102+
logger.Debug("FFmpeg process terminated gracefully")
103+
case <-time.After(3 * time.Second):
104+
logger.Debug("FFmpeg process did not terminate gracefully, forcing kill")
105+
// Force kill if it doesn't terminate gracefully
106+
if err := s.ffmpegCmd.Process.Kill(); err != nil {
107+
logger.Warn("Failed to kill FFmpeg process", zap.Error(err))
108+
}
109+
// Wait a bit more for the kill to take effect
110+
select {
111+
case <-done:
112+
case <-time.After(2 * time.Second):
113+
logger.Warn("FFmpeg process may not have been killed properly")
114+
}
115+
}
116+
}
117+
118+
// Kill any remaining child processes in the process group
119+
if pgid, err := syscall.Getpgid(s.ffmpegCmd.Process.Pid); err == nil {
120+
logger.Debug("Killing process group", zap.Int("pgid", pgid))
121+
// Kill entire process group to ensure all child processes are terminated
122+
syscall.Kill(-pgid, syscall.SIGKILL)
90123
}
91124
}
92125

93-
// Cancel Chrome context
126+
// Cancel Chrome context with timeout
94127
if s.chromeCancel != nil {
95128
logger.Debug("Cancelling Chrome context")
96129
s.chromeCancel()
130+
// Give Chrome context time to clean up
131+
time.Sleep(1 * time.Second)
97132
}
98133

99134
// Cancel main stream context
@@ -179,6 +214,9 @@ func main() {
179214
zap.Int("width", config.Width),
180215
zap.Int("height", config.Height))
181216

217+
// Log initial memory stats
218+
logMemoryStats(logger)
219+
182220
ctx, cancel := context.WithCancel(ctx)
183221
defer cancel()
184222

@@ -225,6 +263,7 @@ func main() {
225263
}()
226264

227265
// Run the stream in a loop to handle restarts from the cron job or manual restarts
266+
var restartCount int
228267
for {
229268
select {
230269
// Case to handle an expected shutdown signal
@@ -234,7 +273,20 @@ func main() {
234273
// Default case to start or restart the stream
235274
// This will be triggered by the cron job or manual restarts
236275
default:
237-
logger.Info("Starting/restarting stream...")
276+
logger.Info("Starting/restarting stream...", zap.Int("restartCount", restartCount))
277+
278+
// Perform periodic cleanup every 10 restarts to prevent resource accumulation
279+
if restartCount > 0 && restartCount%10 == 0 {
280+
logger.Info("Performing periodic cleanup", zap.Int("restartCount", restartCount))
281+
logMemoryStats(logger)
282+
cleanupChromeProcesses(logger)
283+
runtime.GC()
284+
// Give system time to cleanup
285+
time.Sleep(2 * time.Second)
286+
logger.Info("Periodic cleanup completed")
287+
logMemoryStats(logger)
288+
}
289+
238290
if err := streamWebpage(ctx, config); err != nil {
239291
if ctx.Err() != nil {
240292
logger.Info("Stream stopped due to context cancellation")
@@ -243,6 +295,7 @@ func main() {
243295
logger.Info("Stream ended, will restart in 5 seconds", zap.Error(err))
244296
time.Sleep(5 * time.Second)
245297
}
298+
restartCount++
246299
}
247300
}
248301
}
@@ -324,6 +377,10 @@ func streamWebpage(ctx context.Context, config *Config) error {
324377
time.Sleep(2 * time.Second)
325378
}
326379

380+
// Force garbage collection before starting new stream to free up memory
381+
runtime.GC()
382+
logger.Debug("Forced garbage collection before starting new stream")
383+
327384
// Create a cancellable context for this stream session
328385
streamCtx, streamCancel := context.WithCancel(ctx)
329386
defer streamCancel()
@@ -347,14 +404,32 @@ func streamWebpage(ctx context.Context, config *Config) error {
347404
chromedp.Flag("disable-blink-features", "AutomationControlled"),
348405
chromedp.Flag("mute-audio", false),
349406
chromedp.Flag("window-position", "0,0"),
407+
// Add memory management flags for Chrome
408+
chromedp.Flag("max_old_space_size", "512"),
409+
chromedp.Flag("memory-pressure-off", true),
410+
chromedp.Flag("disable-background-timer-throttling", true),
411+
chromedp.Flag("disable-renderer-backgrounding", true),
412+
chromedp.Flag("disable-backgrounding-occluded-windows", true),
350413
chromedp.WindowSize(config.Width, config.Height),
351414
)
352415

353416
allocCtx, allocCancel := chromedp.NewExecAllocator(streamCtx, opts...)
354-
defer allocCancel()
417+
defer func() {
418+
logger.Debug("Cancelling Chrome allocator context")
419+
allocCancel()
420+
// Give allocator time to clean up
421+
time.Sleep(500 * time.Millisecond)
422+
}()
355423

356424
chromeCtx, chromeCancel := chromedp.NewContext(allocCtx)
357-
defer chromeCancel()
425+
defer func() {
426+
logger.Debug("Cancelling Chrome context")
427+
chromeCancel()
428+
// Give Chrome context time to clean up
429+
time.Sleep(500 * time.Millisecond)
430+
// Force cleanup of any remaining Chrome processes
431+
cleanupChromeProcesses(logger)
432+
}()
358433

359434
// Start Chrome and navigate to webpage
360435
logger.Info("Starting Chrome browser", zap.String("url", config.WebpageURL))
@@ -381,6 +456,50 @@ func streamWebpage(ctx context.Context, config *Config) error {
381456
return startFFmpegStream(streamCtx, config, displayInfo, streamCancel, chromeCancel)
382457
}
383458

459+
// Function to clean up any remaining Chrome processes
460+
func cleanupChromeProcesses(logger *zap.Logger) {
461+
logger.Debug("Cleaning up Chrome processes")
462+
463+
// Kill any remaining chrome processes
464+
cmd := exec.Command("pkill", "-f", "chrome")
465+
if err := cmd.Run(); err != nil {
466+
logger.Debug("No Chrome processes to cleanup or cleanup failed", zap.Error(err))
467+
}
468+
469+
// Force kill any stubborn chrome processes
470+
cmd = exec.Command("pkill", "-9", "-f", "chrome")
471+
if err := cmd.Run(); err != nil {
472+
logger.Debug("No Chrome processes to force kill or force kill failed", zap.Error(err))
473+
}
474+
475+
// Also cleanup chromium processes (in case they exist)
476+
cmd = exec.Command("pkill", "-f", "chromium")
477+
if err := cmd.Run(); err != nil {
478+
logger.Debug("No Chromium processes to cleanup or cleanup failed", zap.Error(err))
479+
}
480+
481+
logger.Debug("Chrome process cleanup completed")
482+
}
483+
484+
// Function to log memory usage statistics for monitoring
485+
func logMemoryStats(logger *zap.Logger) {
486+
var m runtime.MemStats
487+
runtime.ReadMemStats(&m)
488+
489+
logger.Info("Memory statistics",
490+
zap.Uint64("allocated_mb", bToMb(m.Alloc)),
491+
zap.Uint64("total_allocated_mb", bToMb(m.TotalAlloc)),
492+
zap.Uint64("heap_objects", m.HeapObjects),
493+
zap.Uint32("num_gc", m.NumGC),
494+
zap.Uint64("sys_mb", bToMb(m.Sys)),
495+
)
496+
}
497+
498+
// Helper function to convert bytes to megabytes
499+
func bToMb(b uint64) uint64 {
500+
return b / 1024 / 1024
501+
}
502+
384503
// Function to get the display info generated by the start.sh script and feed it to FFmpeg
385504
func getDisplayInfo() (string, error) {
386505
// Try to get the DISPLAY environment variable
@@ -490,6 +609,9 @@ func startFFmpegStream(ctx context.Context, config *Config, display string, stre
490609
cmd := exec.CommandContext(ctx, "ffmpeg", args...)
491610
cmd.Stdout = zapWriter
492611
cmd.Stderr = zapWriter
612+
613+
// Set process group ID to allow killing all child processes
614+
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
493615

494616
logger.Debug("Starting FFmpeg with command", zap.Strings("args", args))
495617

@@ -513,6 +635,10 @@ func startFFmpegStream(ctx context.Context, config *Config, display string, stre
513635
globalStreamState.chromeCancel = nil
514636
globalStreamState.ffmpegCmd = nil
515637
globalStreamState.mu.Unlock()
638+
639+
// Force garbage collection after stream ends
640+
runtime.GC()
641+
logger.Debug("Forced garbage collection after stream ended")
516642
}()
517643

518644
if ctx.Err() != nil {

cmd/main_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"net/http"
77
"net/http/httptest"
88
"os/exec"
9+
"syscall"
910
"testing"
1011

1112
"go.uber.org/zap"
@@ -363,3 +364,47 @@ func TestGetDisplayInfo(t *testing.T) {
363364
}
364365
})
365366
}
367+
368+
func TestCleanupChromeProcesses(t *testing.T) {
369+
t.Run("Chrome Process Cleanup Function", func(t *testing.T) {
370+
logger, _ := zap.NewDevelopment()
371+
372+
// This test just ensures the function doesn't crash
373+
// In a real environment, it would clean up Chrome processes
374+
cleanupChromeProcesses(logger)
375+
376+
// If we reach here, the function completed without panicking
377+
// which is what we want to verify
378+
})
379+
}
380+
381+
func TestStopStreamWithProcessGroup(t *testing.T) {
382+
t.Cleanup(resetGlobalStreamState)
383+
384+
t.Run("Stop Stream With Process Group Cleanup", func(t *testing.T) {
385+
logger, _ := zap.NewDevelopment()
386+
387+
// Create a mock command (sleep command that won't actually run long)
388+
cmd := exec.Command("sleep", "0.1")
389+
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
390+
cmd.Start()
391+
392+
// Set up stream state with the mock command
393+
globalStreamState.mu.Lock()
394+
globalStreamState.isRunning = true
395+
globalStreamState.ffmpegCmd = cmd
396+
globalStreamState.mu.Unlock()
397+
398+
// Test that stopStream can handle process group cleanup
399+
globalStreamState.stopStream(logger)
400+
401+
// Verify the stream state is cleaned up
402+
if globalStreamState.isStreamRunning() {
403+
t.Error("Expected stream to be stopped")
404+
}
405+
406+
if globalStreamState.ffmpegCmd != nil {
407+
t.Error("Expected ffmpegCmd to be nil after stopping")
408+
}
409+
})
410+
}

start.sh

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,5 +105,16 @@ done
105105
echo "✓ PulseAudio is responding"
106106

107107
echo "=== All Dependencies Ready - Starting Application ==="
108+
109+
# Set memory management environment variables
110+
export GOMEMLIMIT="512MiB" # Limit Go memory usage
111+
export GOGC=100 # Standard garbage collection target
112+
113+
# Cleanup any existing processes to free memory
114+
echo "Cleaning up any existing processes..."
115+
pkill -f chrome 2>/dev/null || true
116+
pkill -f chromium 2>/dev/null || true
117+
pkill -f ffmpeg 2>/dev/null || true
118+
108119
# Start the stream application
109120
exec /stream

stream

-15.9 MB
Binary file not shown.

0 commit comments

Comments
 (0)