fix: avoid locked player state callbacks

commit: 2f38a8b9a3d9656206af0da00d7c5231492e4733
author: brice · 6mo ago · 2026-04-09 14:59 UTC
parent: 6cf8586
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +47 -5

M internal/core/player.go +10 -5


@@ -15,7 +15,7 @@ type playerConfig struct {
15	15
16	16		// playerCallbacks holds callbacks for player events
17	17		type playerCallbacks struct {
18		-	OnStateChange func(oldState, newState PlayerState)
	18	+	OnStateChange func(PlayerState)
19	19		}
20	20
21	21		// player is the internal player shell with state machine

@@ -50,21 +50,26 @@ func (p *player) setState(next PlayerState) {
50	50		// transition attempts to transition to the next state
51	51		func (p *player) transition(next PlayerState) error {
52	52		p.mu.Lock()
53		-	defer p.mu.Unlock()
54	53
55	54		current := p.State()
56	55
57	56		// Validate transition
58	57		if !isValidTransition(current, next) {
	58	+	p.mu.Unlock()
59	59		return ErrInvalidPlayerState
60	60		}
61	61
62	62		// Perform transition
63	63		p.setState(next)
64	64
65		-	// Notify callback
66		-	if p.callbacks.OnStateChange != nil {
67		-	p.callbacks.OnStateChange(current, next)
	65	+	// Capture callback under lock
	66	+	cb := p.callbacks.OnStateChange
	67	+
	68	+	p.mu.Unlock()
	69	+
	70	+	// Invoke callback after releasing lock to avoid deadlock on reentry
	71	+	if cb != nil {
	72	+	cb(next)
68	73		}
69	74
70	75		return nil


@@ -15,7 +15,7 @@ type playerConfig struct {
15			15
16		// playerCallbacks holds callbacks for player events	16		// playerCallbacks holds callbacks for player events
17		type playerCallbacks struct {	17		type playerCallbacks struct {
18	-	OnStateChange func(oldState, newState PlayerState)	18	+	OnStateChange func(PlayerState)
19		}	19		}
20			20
21		// player is the internal player shell with state machine	21		// player is the internal player shell with state machine

@@ -50,21 +50,26 @@ func (p *player) setState(next PlayerState) {
50		// transition attempts to transition to the next state	50		// transition attempts to transition to the next state
51		func (p *player) transition(next PlayerState) error {	51		func (p *player) transition(next PlayerState) error {
52		p.mu.Lock()	52		p.mu.Lock()
53	-	defer p.mu.Unlock()
54			53
55		current := p.State()	54		current := p.State()
56			55
57		// Validate transition	56		// Validate transition
58		if !isValidTransition(current, next) {	57		if !isValidTransition(current, next) {
			58	+	p.mu.Unlock()
59		return ErrInvalidPlayerState	59		return ErrInvalidPlayerState
60		}	60		}
61			61
62		// Perform transition	62		// Perform transition
63		p.setState(next)	63		p.setState(next)
64			64
65	-	// Notify callback	65	+	// Capture callback under lock
66	-	if p.callbacks.OnStateChange != nil {	66	+	cb := p.callbacks.OnStateChange
67	-	p.callbacks.OnStateChange(current, next)	67	+
			68	+	p.mu.Unlock()
			69	+
			70	+	// Invoke callback after releasing lock to avoid deadlock on reentry
			71	+	if cb != nil {
			72	+	cb(next)
68		}	73		}
69			74
70		return nil	75		return nil

M internal/core/player_state_test.go +37 -0


@@ -4,6 +4,43 @@ import (
4	4		"testing"
5	5		)
6	6
	7	+	func TestPlayerStateCallbackFanout(t *testing.T) {
	8	+	var callbackStates []PlayerState
	9	+	callbacks := playerCallbacks{
	10	+	OnStateChange: func(state PlayerState) {
	11	+	callbackStates = append(callbackStates, state)
	12	+	},
	13	+	}
	14	+
	15	+	p := newPlayer(playerConfig{}, callbacks)
	16	+
	17	+	// Trigger a transition
	18	+	if err := p.transition(PlayerStateStarting); err != nil {
	19	+	t.Errorf("transition to Starting failed: %v", err)
	20	+	}
	21	+
	22	+	// Verify callback was invoked with the new state
	23	+	if len(callbackStates) != 1 {
	24	+	t.Errorf("callback invoked %d times, want 1", len(callbackStates))
	25	+	}
	26	+	if len(callbackStates) > 0 && callbackStates[0] != PlayerStateStarting {
	27	+	t.Errorf("callback received state %v, want %v", callbackStates[0], PlayerStateStarting)
	28	+	}
	29	+
	30	+	// Trigger another transition
	31	+	if err := p.transition(PlayerStatePlaying); err != nil {
	32	+	t.Errorf("transition to Playing failed: %v", err)
	33	+	}
	34	+
	35	+	// Verify callback was invoked again
	36	+	if len(callbackStates) != 2 {
	37	+	t.Errorf("callback invoked %d times, want 2", len(callbackStates))
	38	+	}
	39	+	if len(callbackStates) > 1 && callbackStates[1] != PlayerStatePlaying {
	40	+	t.Errorf("callback received state %v, want %v", callbackStates[1], PlayerStatePlaying)
	41	+	}
	42	+	}
	43	+
7	44		func TestPlayerStateTransitionsIdleStartStopClose(t *testing.T) {
8	45		p := newPlayer(playerConfig{}, playerCallbacks{})
9	46


@@ -4,6 +4,43 @@ import (
4	"testing"	4		"testing"
5	)	5		)
6		6
		7	+	func TestPlayerStateCallbackFanout(t *testing.T) {
		8	+	var callbackStates []PlayerState
		9	+	callbacks := playerCallbacks{
		10	+	OnStateChange: func(state PlayerState) {
		11	+	callbackStates = append(callbackStates, state)
		12	+	},
		13	+	}
		14	+
		15	+	p := newPlayer(playerConfig{}, callbacks)
		16	+
		17	+	// Trigger a transition
		18	+	if err := p.transition(PlayerStateStarting); err != nil {
		19	+	t.Errorf("transition to Starting failed: %v", err)
		20	+	}
		21	+
		22	+	// Verify callback was invoked with the new state
		23	+	if len(callbackStates) != 1 {
		24	+	t.Errorf("callback invoked %d times, want 1", len(callbackStates))
		25	+	}
		26	+	if len(callbackStates) > 0 && callbackStates[0] != PlayerStateStarting {
		27	+	t.Errorf("callback received state %v, want %v", callbackStates[0], PlayerStateStarting)
		28	+	}
		29	+
		30	+	// Trigger another transition
		31	+	if err := p.transition(PlayerStatePlaying); err != nil {
		32	+	t.Errorf("transition to Playing failed: %v", err)
		33	+	}
		34	+
		35	+	// Verify callback was invoked again
		36	+	if len(callbackStates) != 2 {
		37	+	t.Errorf("callback invoked %d times, want 2", len(callbackStates))
		38	+	}
		39	+	if len(callbackStates) > 1 && callbackStates[1] != PlayerStatePlaying {
		40	+	t.Errorf("callback received state %v, want %v", callbackStates[1], PlayerStatePlaying)
		41	+	}
		42	+	}
		43	+
7	func TestPlayerStateTransitionsIdleStartStopClose(t *testing.T) {	44		func TestPlayerStateTransitionsIdleStartStopClose(t *testing.T) {
8	p := newPlayer(playerConfig{}, playerCallbacks{})	45		p := newPlayer(playerConfig{}, playerCallbacks{})
9		46