From b4dbf1e109d4886c464a19d9f666d9bfab743c60 Mon Sep 17 00:00:00 2001
From: Gabe Cook <gabe565@gmail.com>
Date: Sun, 28 Jul 2024 04:49:06 -0500
Subject: [PATCH] fix(player): Prevent slight memory leak by ensuring player is
 always closed

---
 .golangci.yaml            |  5 -----
 cmd/play/cmd.go           |  6 ++++--
 internal/player/events.go |  6 ------
 internal/player/player.go | 25 +++++++++++++------------
 internal/server/ssh.go    |  8 ++++++--
 internal/server/telnet.go |  4 +++-
 6 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/.golangci.yaml b/.golangci.yaml
index 27fc818b..a995af40 100644
--- a/.golangci.yaml
+++ b/.golangci.yaml
@@ -8,11 +8,6 @@ linters-settings:
   gosec:
     excludes:
       - G306
-  forbidigo:
-    forbid:
-      - "^(fmt\\.Print(|f|ln)|print|println)$"
-      - p: "^tea\\.Quit$"
-        msg: tea.Quit may cause a memory leak
 
 linters:
   enable:
diff --git a/cmd/play/cmd.go b/cmd/play/cmd.go
index dd6d781c..c94f5f57 100644
--- a/cmd/play/cmd.go
+++ b/cmd/play/cmd.go
@@ -40,8 +40,10 @@ func run(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	program := tea.NewProgram(
-		player.NewPlayer(&m, log.Level(zerolog.ErrorLevel), nil),
+	p := player.NewPlayer(&m, log.Level(zerolog.ErrorLevel), nil)
+	defer p.Close()
+
+	program := tea.NewProgram(p,
 		tea.WithAltScreen(),
 		tea.WithMouseCellMotion(),
 	)
diff --git a/internal/player/events.go b/internal/player/events.go
index 78b4885a..ddc60c7b 100644
--- a/internal/player/events.go
+++ b/internal/player/events.go
@@ -19,9 +19,3 @@ func tick(ctx context.Context, d time.Duration, msg tea.Msg) tea.Cmd {
 		}
 	}
 }
-
-type quitMsg struct{}
-
-func Quit() tea.Msg {
-	return quitMsg{}
-}
diff --git a/internal/player/player.go b/internal/player/player.go
index 46f12088..fa9985bc 100644
--- a/internal/player/player.go
+++ b/internal/player/player.go
@@ -75,7 +75,7 @@ func (p *Player) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		case p.speed >= 0:
 			frameDiff = 1
 			if p.frame+frameDiff >= len(p.movie.Frames) {
-				return p, Quit
+				return p, tea.Quit
 			}
 		case p.frame <= 0:
 			p.speed = 1
@@ -93,7 +93,7 @@ func (p *Player) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		duration := p.movie.Frames[p.frame].CalcDuration(speed)
 		for duration < time.Second/15 {
 			if p.frame+frameDiff >= len(p.movie.Frames) {
-				return p, Quit
+				return p, tea.Quit
 			} else if p.frame+frameDiff <= 0 {
 				p.speed = 1
 				p.activeOption = 4
@@ -108,7 +108,7 @@ func (p *Player) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		p.optionViewStale = true
 		switch {
 		case key.Matches(msg, p.keymap.quit):
-			return p, Quit
+			return p, tea.Quit
 		case key.Matches(msg, p.keymap.left):
 			if p.selectedOption > 0 {
 				p.selectedOption--
@@ -164,15 +164,6 @@ func (p *Player) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 				}
 			}
 		}
-	case quitMsg:
-		if p.frame >= len(p.movie.Frames)-1 {
-			p.log.Info().Msg("Finished movie")
-		} else {
-			p.log.Info().Msg("Disconnected early")
-		}
-		p.clearTimeouts()
-		p.zone.Close()
-		return p, tea.Quit //nolint:forbidigo
 	case Option:
 		p.optionViewStale = true
 		switch msg {
@@ -352,3 +343,13 @@ func (p *Player) clearTimeouts() {
 		p.playCancel()
 	}
 }
+
+func (p *Player) Close() {
+	if p.frame >= len(p.movie.Frames)-1 {
+		p.log.Info().Msg("Finished movie")
+	} else {
+		p.log.Info().Msg("Disconnected early")
+	}
+	p.clearTimeouts()
+	p.zone.Close()
+}
diff --git a/internal/server/ssh.go b/internal/server/ssh.go
index c31dd4ef..e06444c3 100644
--- a/internal/server/ssh.go
+++ b/internal/server/ssh.go
@@ -136,8 +136,12 @@ func (s *SSHServer) Handler(m *movie.Movie) bubbletea.Handler {
 			}
 		}
 
-		player := player.NewPlayer(m, logger, renderer)
-		return player, []tea.ProgramOption{
+		p := player.NewPlayer(m, logger, renderer)
+		go func() {
+			<-session.Context().Done()
+			p.Close()
+		}()
+		return p, []tea.ProgramOption{
 			tea.WithFPS(30),
 			tea.WithAltScreen(),
 			tea.WithMouseCellMotion(),
diff --git a/internal/server/telnet.go b/internal/server/telnet.go
index e370f644..74fe342d 100644
--- a/internal/server/telnet.go
+++ b/internal/server/telnet.go
@@ -136,6 +136,8 @@ func (s *TelnetServer) Handler(ctx context.Context, conn net.Conn, m *movie.Movi
 	}
 
 	p := player.NewPlayer(m, logger, telnet.MakeRenderer(outW, profile))
+	defer p.Close()
+
 	opts := []tea.ProgramOption{
 		tea.WithInput(inR),
 		tea.WithOutput(outW),
@@ -157,7 +159,7 @@ func (s *TelnetServer) Handler(ctx context.Context, conn net.Conn, m *movie.Movi
 					})
 				}
 			case <-ctx.Done():
-				program.Send(player.Quit())
+				program.Send(tea.Quit())
 				return
 			}
 		}