-
-
Notifications
You must be signed in to change notification settings - Fork 247
Disconnect sendspin clients to allow clean shutdown #2887
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
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 shutdown hang issue in the Sendspin provider when clients are connected. The fix ensures that Music Assistant can perform a clean shutdown (e.g., via Ctrl+C) even when Sendspin clients are actively connected.
Key Changes:
- Adds graceful client disconnection in the
unloadmethod before stopping the server
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@maximmaxim345 Could you add the 'backport to stable' label if this can safely go into the next stable patch release? |
On shutdown MA already unregisters all players before unloading the provider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
I was unable to do a clean shutdown in development when a Sendspin client was connected to the Sendspin server. This PR fixes it. STR - Start MA - Connect sendspin client (I was using sendspin-js) - Shut down MA using ctrl+C - Hangs With this PR: shuts down cleanly
I was unable to do a clean shutdown in development when a Sendspin client was connected to the Sendspin server. This PR fixes it. STR - Start MA - Connect sendspin client (I was using sendspin-js) - Shut down MA using ctrl+C - Hangs With this PR: shuts down cleanly
Log each client disconnect attempt and warn on failures. From music-assistant/server#2887
I was unable to do a clean shutdown in development when a Sendspin client was connected to the Sendspin server. This PR fixes it.
STR
With this PR: shuts down cleanly