Files
yellowjacket/.planning/phases/01-concurrency-race-fixes/01-01-PLAN.md
T

9.7 KiB

phase, plan, type, wave, depends_on, files_modified, autonomous, requirements, must_haves
phase plan type wave depends_on files_modified autonomous requirements must_haves
01-concurrency-race-fixes 01 execute 1
backend/queue/queue.go
backend/library/library.go
backend/playlist/playlist.go
backend/player/player.go
true
CORR-01
CORR-02
CORR-03
CORR-04
truths artifacts key_links
Queue.SetContext() acquires q.mu before writing q.ctx
Library.SetContext() and SetRescanHooks() acquire a mutex before writing fields
Playlist.Service.SetContext() acquires a mutex before writing s.ctx
Player.SetContext() uses a single lock acquisition instead of double-lock
Running go test -race on all four packages produces zero data race reports for SetContext
path provides contains
backend/queue/queue.go Race-free Queue.SetContext q.mu.Lock
path provides contains
backend/library/library.go Race-free Library.SetContext and SetRescanHooks with struct-level mutex l.mu.Lock
path provides contains
backend/playlist/playlist.go Race-free Service.SetContext with struct-level mutex s.mu.Lock
path provides contains
backend/player/player.go Single-lock Player.SetContext p.restoreStateLocked
from to via pattern
backend/queue/queue.go:SetContext backend/queue/emit.go:emitQueueChanged Both read q.ctx under q.mu q.mu.Lock.*q.ctx
from to via pattern
backend/library/library.go:SetContext backend/library/library.go:registerEventHandlers SetContext acquires l.mu then calls registerEventHandlers after release l.mu.Lock.*l.ctx
from to via pattern
backend/playlist/playlist.go:SetContext backend/playlist/playlist.go:emitEvent Both access s.ctx under s.mu s.mu.Lock.*s.ctx
Eliminate all SetContext data races across Queue, Library, Playlist, and Player packages.

Purpose: These four SetContext methods write struct fields without proper synchronization, creating data races detectable by go test -race. Fixing them makes the codebase race-clean for all subsequent test phases.

Output: Four modified Go files with mutex-protected SetContext implementations.

<execution_context> @/home/caleb/.config/opencode/get-shit-done/workflows/execute-plan.md @/home/caleb/.config/opencode/get-shit-done/templates/summary.md </execution_context>

@.planning/ROADMAP.md @.planning/STATE.md @.planning/codebase/CONVENTIONS.md @.planning/codebase/CONCERNS.md

@backend/queue/queue.go @backend/queue/emit.go @backend/library/library.go @backend/playlist/playlist.go @backend/player/player.go

From backend/queue/queue.go (lines 104-122):

type Queue struct {
	ctx    context.Context
	logger *slog.Logger
	db     *database.DB
	player TrackLoader

	mu               sync.Mutex
	tracks           []Track
	currentIndex     int
	shuffleMode      bool
	repeatMode       RepeatMode
	shuffleOrder     []int
	sourcePlaylistID int64

	setQueueGen atomic.Int64
}

From backend/library/library.go (lines 77-84):

type Library struct {
	ctx         context.Context
	logger      *slog.Logger
	conf        *Config
	db          *database.DB
	rescanHooks RescanHooks
}
// NOTE: No struct-level mutex exists. Must add one.

From backend/playlist/playlist.go (lines 97-104):

type Service struct {
	ctx           context.Context
	logger        *slog.Logger
	db            *database.DB
	libraryDir    LibraryDirProvider
	favoritesConf FavoritesConfigProvider
}
// NOTE: No mutex exists. Must add one.

From backend/player/player.go (lines 30-40, 163-171):

type Player struct {
	mu sync.Mutex
	ctx context.Context
	// ... other fields
}

// Current double-lock SetContext:
func (p *Player) SetContext(ctx context.Context) {
	p.mu.Lock()
	p.ctx = ctx
	p.mu.Unlock()

	p.mu.Lock()
	p.restoreStateLocked()
	p.mu.Unlock()
}

Codebase mutex convention (from CONVENTIONS.md):

// Public method acquires lock:
func (p *Player) Play() error {
	p.mu.Lock()
	defer p.mu.Unlock()
	// ...
}

// Internal helper — caller must hold p.mu:
func (p *Player) loadFileLocked(filePath string) error {
	// no lock acquired here
}
Task 1: Add mutex protection to Queue, Library, and Playlist SetContext methods backend/queue/queue.go backend/library/library.go backend/playlist/playlist.go **Queue (backend/queue/queue.go):**

In SetContext() (line 134), wrap the q.ctx = ctx assignment with the existing q.mu:

func (q *Queue) SetContext(ctx context.Context) {
	q.mu.Lock()
	defer q.mu.Unlock()

	q.ctx = ctx
}

No other changes needed — q.mu already exists in the struct, and all emit methods that read q.ctx are called from methods that hold q.mu.

Library (backend/library/library.go):

  1. Add a mu sync.Mutex field to the Library struct (line 78 area), placed as the first field to follow the player convention. Add a doc comment explaining it protects ctx, conf, and rescanHooks.

  2. Update SetContext() (line 120) to acquire l.mu before writing l.ctx, then release before calling l.registerEventHandlers() (which itself calls runtime.EventsOn — should not hold the mutex during potentially blocking Wails calls):

func (l *Library) SetContext(ctx context.Context) {
	l.mu.Lock()
	l.ctx = ctx
	l.mu.Unlock()

	l.registerEventHandlers()
}
  1. Update SetRescanHooks() (line 88) to acquire l.mu:
func (l *Library) SetRescanHooks(h RescanHooks) {
	l.mu.Lock()
	defer l.mu.Unlock()

	l.rescanHooks = h
}

Do NOT add mutex to scan-internal paths — the scan methods run single-threaded after startup. Only protect the fields that are written by setter methods called during initialization.

Playlist (backend/playlist/playlist.go):

  1. Add a mu sync.Mutex field to the Service struct (line 98 area), placed before ctx. Import "sync" if not already imported.

  2. Update SetContext() (line 130) to acquire s.mu before writing s.ctx, then release before calling s.migrateExistingPlaylists():

func (s *Service) SetContext(ctx context.Context) {
	s.mu.Lock()
	s.ctx = ctx
	s.mu.Unlock()

	s.migrateExistingPlaylists()
}
  1. Update SetFavoritesConfig() (line 121) to acquire s.mu:
func (s *Service) SetFavoritesConfig(
	provider FavoritesConfigProvider,
) {
	s.mu.Lock()
	defer s.mu.Unlock()

	s.favoritesConf = provider
}

For all three packages: follow existing codebase conventions — sync.Mutex named mu, Lock()/defer Unlock() for simple setters, explicit Lock()/Unlock() when code after the critical section should run without the lock. cd /mnt/vault/dev/golang/yellowjacket && go build ./backend/queue/ ./backend/library/ ./backend/playlist/ - Queue.SetContext acquires q.mu before writing q.ctx - Library struct has a mu sync.Mutex field; SetContext and SetRescanHooks acquire it - Playlist Service struct has a mu sync.Mutex field; SetContext and SetFavoritesConfig acquire it - All three packages compile without errors

Task 2: Collapse Player.SetContext double-lock into single acquisition backend/player/player.go Replace the current double-lock `SetContext()` (lines 163-171):
func (p *Player) SetContext(ctx context.Context) {
	p.mu.Lock()
	p.ctx = ctx
	p.mu.Unlock()

	p.mu.Lock()
	p.restoreStateLocked()
	p.mu.Unlock()
}

With a single lock acquisition:

func (p *Player) SetContext(ctx context.Context) {
	p.mu.Lock()
	defer p.mu.Unlock()

	p.ctx = ctx
	p.restoreStateLocked()
}

This is safe because restoreStateLocked() is documented as requiring p.mu to be held (the Locked suffix convention), and combining the operations prevents another goroutine from observing a partially-initialized state (ctx set but state not yet restored).

WARNING: Do NOT change any other Player methods. Do NOT alter lock ordering between p.mu and speaker.Lock(). The player's lock-sensitive paths are fragile and this change is scoped only to SetContext. cd /mnt/vault/dev/golang/yellowjacket && go build ./backend/player/ - Player.SetContext uses a single p.mu.Lock()/defer p.mu.Unlock() call - p.ctx assignment and p.restoreStateLocked() both run under the same lock hold - Player package compiles without errors

After both tasks complete, run the full verification:
# 1. All four packages compile
go build ./backend/queue/ ./backend/library/ ./backend/playlist/ ./backend/player/

# 2. Existing tests still pass (with race detector)
go test -race -count=1 ./backend/player/ ./backend/playlist/ ./backend/coverart/ ./backend/metadata/...

# 3. Vet passes on modified packages
go vet ./backend/queue/ ./backend/library/ ./backend/playlist/ ./backend/player/

# 4. Lint passes (if golangci-lint available)
golangci-lint run ./backend/queue/ ./backend/library/ ./backend/playlist/ ./backend/player/

<success_criteria>

  1. All four SetContext methods acquire their respective mutex before writing the ctx field
  2. Library and Playlist structs have new mu sync.Mutex fields
  3. Player.SetContext uses exactly one Lock/Unlock pair instead of two
  4. go build succeeds on all four packages
  5. go test -race on existing test files produces zero race reports
  6. go vet reports no issues on modified packages </success_criteria>
After completion, create `.planning/phases/01-concurrency-race-fixes/01-01-SUMMARY.md`