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 |
|
true |
|
|
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
}
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):
-
Add a
mu sync.Mutexfield to theLibrarystruct (line 78 area), placed as the first field to follow the player convention. Add a doc comment explaining it protectsctx,conf, andrescanHooks. -
Update
SetContext()(line 120) to acquirel.mubefore writingl.ctx, then release before callingl.registerEventHandlers()(which itself callsruntime.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()
}
- Update
SetRescanHooks()(line 88) to acquirel.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):
-
Add a
mu sync.Mutexfield to theServicestruct (line 98 area), placed beforectx. Import"sync"if not already imported. -
Update
SetContext()(line 130) to acquires.mubefore writings.ctx, then release before callings.migrateExistingPlaylists():
func (s *Service) SetContext(ctx context.Context) {
s.mu.Lock()
s.ctx = ctx
s.mu.Unlock()
s.migrateExistingPlaylists()
}
- Update
SetFavoritesConfig()(line 121) to acquires.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
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
# 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>
- All four SetContext methods acquire their respective mutex before writing the ctx field
- Library and Playlist structs have new
mu sync.Mutexfields - Player.SetContext uses exactly one Lock/Unlock pair instead of two
go buildsucceeds on all four packagesgo test -raceon existing test files produces zero race reportsgo vetreports no issues on modified packages </success_criteria>