Skip to content

Fix sync group member playing out of sync after concurrent group changes#4189

Merged
marcelveldt merged 1 commit into
devfrom
fix/syncgroup-leader-playback-wait
Jun 12, 2026
Merged

Fix sync group member playing out of sync after concurrent group changes#4189
marcelveldt merged 1 commit into
devfrom
fix/syncgroup-leader-playback-wait

Conversation

@marcelveldt

Copy link
Copy Markdown
Member

What does this implement/fix?

A sync group member could end up playing the group's content on its own — the same audio on the speaker but several seconds out of sync, and not shown as part of the group in the UI.

A leader change dissolves and re-forms the group, which promotes a new leader and starts it playing. play()/play_media() returned as soon as the play command was dispatched to the device — before it actually started — so the group's playback lock was released while the start was still in flight. When a second membership command arrives almost simultaneously (e.g. Home Assistant powering off the current leader while ungrouping another member), it acquires the lock next and dissolves the group, racing the in-flight start: the play lands at the device after the stop, stranding that player streaming while the group's logical state has already moved on without it. Re-joining the player resyncs it.

The fix makes the (re)start synchronous within the lock, mirroring the stop/dissolve side which already waits for the player to settle.

Related issue (if applicable):

  • N/A

Changes

  • Add a _await_leader_playback() context manager; play() and play_media() wrap the leader's resume/play in it, so the group's playback lock stays held until the sync leader actually reports playing (or a short timeout elapses).
  • Add a PLAYBACK_START_TIMEOUT constant for that wait.
  • Add regression tests covering both start paths and the no-leader no-op.

Types of changes

  • Bugfix (non-breaking change which fixes an issue) — bugfix
  • New feature (non-breaking change which adds functionality) — new-feature
  • Enhancement to an existing feature — enhancement
  • New music/player/metadata/plugin provider — new-provider
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) — breaking-change
  • Refactor (no behaviour change) — refactor
  • Documentation only — documentation
  • Maintenance / chore — maintenance
  • CI / workflow change — ci
  • Dependencies bump — dependencies

Checklist

  • The code change is tested and works locally.
  • pre-commit run --all-files passes.
  • pytest passes, and tests have been added/updated under tests/ where applicable.
  • For changes to shared models, the companion PR in music-assistant/models is linked.
  • For changes affecting the UI, the companion PR in music-assistant/frontend is linked.
  • I have read and complied with the project's AI Policy for any AI-assisted contributions.
  • I have raised a PR against the documentation repository targeting the main or beta branch as appropriate.

… changes

A leader change dissolves and re-forms the group, promoting a new leader and
starting it playing. play()/play_media() returned as soon as the play command
was dispatched - before the device actually started - so the group's playback
lock was released while the start was still in flight. A closely-following
command (e.g. HA powering off the old leader while ungrouping another member)
could then race it: the play landed after the stop, leaving that player
streaming while the group's logical state had moved on without it - same audio,
seconds out of sync, not shown as grouped.

Hold the lock until the leader confirms playback so any queued command observes
a settled state, mirroring the existing stop/dissolve side.
Copilot AI review requested due to automatic review settings June 12, 2026 14:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a race in sync group leader (re)starts where the group playback lock could be released before the leader actually begins playing, allowing near-simultaneous (un)group operations to “strand” a member playing out of sync and outside the group’s logical/UI state.

Changes:

  • Adds a _await_leader_playback() async context manager to keep the group playback lock held until the sync leader reports PlaybackState.PLAYING (or times out).
  • Introduces PLAYBACK_START_TIMEOUT to bound the wait duration.
  • Adds regression tests verifying play() and play_media() wrap the leader start/resume with the wait, plus a no-leader no-op case.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
music_assistant/providers/sync_group/player.py Wraps resume/play_media with a leader playback wait context manager to prevent start/stop races during concurrent group changes.
music_assistant/providers/sync_group/constants.py Adds a dedicated timeout constant for awaiting leader playback start confirmation.
tests/providers/test_sync_group.py Adds tests asserting the leader playback wait is armed before issuing start commands and awaited before returning.

@marcelveldt marcelveldt merged commit 49f57ad into dev Jun 12, 2026
12 checks passed
@marcelveldt marcelveldt deleted the fix/syncgroup-leader-playback-wait branch June 12, 2026 14:28
anatosun pushed a commit to anatosun/music-assistant-server that referenced this pull request Jun 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants