Silly walrus

player/player.go

@@ -10,9 +10,8 @@ import (
 // Player handles alarm sound playback by shelling out to system audio tools.
 type Player struct {
 	mu     sync.Mutex
-	cmd     *exec.Cmd
-	playing bool
 	stopCh chan struct{}
+	doneCh chan struct{}
 }
 
 func New() *Player {
@@ -61,104 +60,105 @@ func findPlayer() (string, func(string) []string) {
 
 // PlayLoop starts playing a sound file in a loop until Stop() is called.
 func (p *Player) PlayLoop(path string) {
-	p.mu.Lock()
+	p.Stop() // kill any previous playback and wait for it to finish
-	defer p.mu.Unlock()
 
-	p.stop()
+	p.mu.Lock()
+	p.stopCh = make(chan struct{})
+	p.doneCh = make(chan struct{})
+	stopCh := p.stopCh
+	doneCh := p.doneCh
+	p.mu.Unlock()
 
 	resolved := resolveSound(path)
 	if resolved == "" {
-		// No sound file — bell loop
+		go p.bellLoop(stopCh, doneCh)
-		p.stopCh = make(chan struct{})
-		p.playing = true
-		go func() {
-			ticker := time.NewTicker(2 * time.Second)
-			defer ticker.Stop()
-			os.Stdout.WriteString("\a")
-			for {
-				select {
-				case <-p.stopCh:
-					return
-				case <-ticker.C:
-					os.Stdout.WriteString("\a")
-				}
-			}
-		}()
 		return
 	}
 
 	name, argsFn := findPlayer()
 	if name == "" {
 		os.Stdout.WriteString("\a")
+		close(doneCh)
 		return
 	}
 
-	p.stopCh = make(chan struct{})
+	go p.audioLoop(name, argsFn, resolved, stopCh, doneCh)
-	p.playing = true
+}
+
+// audioLoop runs the audio player in a loop. The goroutine fully owns the
+// process — no other goroutine touches cmd. Coordination is only via channels.
+func (p *Player) audioLoop(name string, argsFn func(string) []string, path string, stopCh, doneCh chan struct{}) {
+	defer close(doneCh)
 
-	go func() {
 	for {
+		// Check if we should stop before starting a new process
 		select {
-			case <-p.stopCh:
+		case <-stopCh:
 			return
 		default:
 		}
 
-			cmd := exec.Command(name, argsFn(resolved)...)
+		cmd := exec.Command(name, argsFn(path)...)
 		cmd.Stdout = nil
 		cmd.Stderr = nil
 
-			p.mu.Lock()
+		if err := cmd.Start(); err != nil {
-			p.cmd = cmd
-			p.mu.Unlock()
-
-			err := cmd.Run()
-
-			p.mu.Lock()
-			p.cmd = nil
-			p.mu.Unlock()
-
-			if err != nil {
-				// Check if we were told to stop
-				select {
-				case <-p.stopCh:
 			return
-				default:
-				}
 		}
 
-			// Brief pause between loops to avoid hammering
+		// Wait for either the process to finish or a stop signal
+		processDone := make(chan error, 1)
+		go func() {
+			processDone <- cmd.Wait()
+		}()
+
 		select {
-			case <-p.stopCh:
+		case <-stopCh:
+			// Kill the process and wait for it to exit
+			_ = cmd.Process.Kill()
+			<-processDone
+			return
+		case <-processDone:
+			// Process finished naturally, loop again after a brief pause
+		}
+
+		// Pause between loops
+		select {
+		case <-stopCh:
 			return
 		case <-time.After(300 * time.Millisecond):
 		}
 	}
-	}()
 }
 
-// Stop stops any currently playing sound.
+func (p *Player) bellLoop(stopCh, doneCh chan struct{}) {
+	defer close(doneCh)
+	ticker := time.NewTicker(2 * time.Second)
+	defer ticker.Stop()
+	os.Stdout.WriteString("\a")
+	for {
+		select {
+		case <-stopCh:
+			return
+		case <-ticker.C:
+			os.Stdout.WriteString("\a")
+		}
+	}
+}
+
+// Stop stops any currently playing sound and waits for the playback goroutine to exit.
 func (p *Player) Stop() {
 	p.mu.Lock()
-	defer p.mu.Unlock()
+	stopCh := p.stopCh
-	p.stop()
+	doneCh := p.doneCh
-}
-
-func (p *Player) stop() {
-	if p.stopCh != nil {
-		close(p.stopCh)
 	p.stopCh = nil
-	}
+	p.doneCh = nil
-	if p.cmd != nil && p.cmd.Process != nil {
+	p.mu.Unlock()
-		_ = p.cmd.Process.Kill()
-		_ = p.cmd.Wait()
-		p.cmd = nil
-	}
-	p.playing = false
-}
 
-func (p *Player) IsPlaying() bool {
+	if stopCh != nil {
-	p.mu.Lock()
+		close(stopCh)
-	defer p.mu.Unlock()
+	}
-	return p.playing
+	if doneCh != nil {
+		<-doneCh // wait for goroutine to actually finish
+	}
 }