spotupnp: fix teardown deadlock when removing a renderer

spotupnp/src/mr_util.c

@@ -126,8 +126,16 @@ void FlushMRDevices(void) {
 		struct sMR *p = &glMRDevices[i];
 		pthread_mutex_lock(&p->Mutex);
 		if (p->Running) {
+			// Same lock-ordering rule as the per-renderer removal paths in
+			// spotupnp.c: do not hold p->Mutex across spotDeletePlayer(), because
+			// the player task's teardown needs that mutex (via shadowRequest) to
+			// make progress, and ~CSpotPlayer() blocks until the task exits.
+			struct spotPlayer *dying = p->SpotPlayer;
+			p->SpotPlayer = NULL;
+			pthread_mutex_unlock(&p->Mutex);
+			spotDeletePlayer(dying);
+			pthread_mutex_lock(&p->Mutex);
 			// device's mutex returns unlocked
-			spotDeletePlayer(p->SpotPlayer);
 			DelMRDevice(p);
 		} else pthread_mutex_unlock(&p->Mutex);
 	}

spotupnp/src/spotify.cpp

@@ -522,6 +522,10 @@ void CSpotPlayer::disconnect(bool abort) {
 }
 
 bool getMetaForUrl(CSpotPlayer* self, const std::string url, metadata_t* metadata) {
+    // Device->Mutex is released around spotDeletePlayer(), during which Device->SpotPlayer
+    // is NULL. Callers such as spotupnp.c's ActionHandler do not null-check, so guard here
+    // to keep NULL a valid sentinel, matching the contract that notify() already upholds.
+    if (!self) return false;
     for (auto it = self->streamers.begin(); it != self->streamers.end(); ++it) {
         if ((*it)->getStreamUrl() == url) {
             (*it)->getMetadata(metadata);

spotupnp/src/spotupnp.c

@@ -802,10 +802,21 @@ static void *UpdateThread(void *args) {
 						// if device does not answer, try to download its DescDoc
 						IXML_Document* DescDoc = NULL;
 						if (UpnpDownloadXmlDoc(Device->DescDocURL, &DescDoc) != UPNP_E_SUCCESS) {
+							struct spotPlayer *dying;
+
 							pthread_mutex_lock(&Device->Mutex);
 							LOG_INFO("[%p]: removing unresponsive player (%s) with error count %d and timeout %d", Device,
 								      Device->Config.Name, Device->ErrorCount, now - Device->LastSeen);
-							spotDeletePlayer(Device->SpotPlayer);
+
+							// spotDeletePlayer blocks in ~CSpotPlayer until the player task exits, and that
+							// task's teardown calls shadowRequest which takes Device->Mutex. Holding the mutex
+							// here would deadlock and leave the renderer gone from Spotify until restart.
+							dying = Device->SpotPlayer;
+							Device->SpotPlayer = NULL;
+							pthread_mutex_unlock(&Device->Mutex);
+							spotDeletePlayer(dying);
+							pthread_mutex_lock(&Device->Mutex);
+
 							// device's mutex returns unlocked
 							DelMRDevice(Device);
 						} else {
@@ -820,14 +831,23 @@ static void *UpdateThread(void *args) {
 
 			// device removal request
 			} else if (Update->Type == BYE_BYE) {
+				struct spotPlayer *dying;
 
 				Device = UDN2Device(Update->Data);
 
 				// Multiple bye-bye might be sent
 				if (!CheckAndLock(Device)) continue;
 
 				LOG_INFO("[%p]: renderer bye-bye: %s", Device, Device->Config.Name);
-				spotDeletePlayer(Device->SpotPlayer);
+
+				// Same lock-ordering rule as the presence-timeout path: drop Device->Mutex
+				// before spotDeletePlayer so the player task can take it and exit cleanly.
+				dying = Device->SpotPlayer;
+				Device->SpotPlayer = NULL;
+				pthread_mutex_unlock(&Device->Mutex);
+				spotDeletePlayer(dying);
+				pthread_mutex_lock(&Device->Mutex);
+
 				// device's mutex returns unlocked
 				DelMRDevice(Device);
 
@@ -885,12 +905,17 @@ static void *UpdateThread(void *args) {
 																  Device->Config.CacheMode, (struct shadowPlayer*) Device, &Device->Mutex);
 							pthread_mutex_unlock(&Device->Mutex);
 						} else if (Master && (!Device->Master || Device->Master == Device)) {
+							struct spotPlayer *dying;
+
 							pthread_mutex_lock(&Device->Mutex);
 							LOG_INFO("[%p]: Sonos %s is now slave", Device, Device->Config.Name);
 							Device->Master = Master;
-							spotDeletePlayer(Device->SpotPlayer);
+
+							// Same lock-ordering rule as above.
+							dying = Device->SpotPlayer;
 							Device->SpotPlayer = NULL;
 							pthread_mutex_unlock(&Device->Mutex);
+							spotDeletePlayer(dying);
 						}
 
 						NFREE(friendlyName);