librespot-org / librespot

Open Source Spotify client library
MIT License
4.75k stars 588 forks source link

Librespot device does not disappear when exiting #676

Open Johannesd3 opened 3 years ago

Johannesd3 commented 3 years ago

The official Spotify clients disappear immediately from the list of available devices in other Spotify clients when they are exitted. Librespot does not disappear.

First I thought it has something to do with the kMessageTypeGoodbye that is sent on shutdown. It doesn't seem to reach another Librespot instance (with log level debug so I can see the incoming messages). But only the web player's goodbye shows up. When I close the desktop client, no goodbye message is shown in Librespot, but it disappears from the web player's device list nevertheless.

I don't really understand what's going on, so maybe someone else can take a look.

This applies to the dev branch as well as to tokio_migration.

roderickvd commented 3 years ago

Maybe the official clients keep a particular port alive to a Spotify server, that librespot doesn't?

Johannesd3 commented 3 years ago

Maybe the official clients keep a particular port alive to a Spotify server, that librespot doesn't?

I believe you understood it the other way round. But the librespot device does not disappear although it should.

roderickvd commented 3 years ago

No I do mean what I said. Perhaps there’s no messaging mechanism but just an established keep-alive connection by which the official clients signal they are available. And on shutdown when they close the connection, that’s their offline signal. Just theorizing here.

Johannesd3 commented 3 years ago

Ah ok. Would seem odd to use an extra connection to signal shutdown, but maybe it's related to the new api.

@devgianlu Does the same issue exist for librespot-java?

devgianlu commented 3 years ago

The new API doesn't seem to have a command to signal shutdown. All it has is these, but I am not having any issue with it not disappearing. I suppose they are detecting the websocket connection closing.

roderickvd commented 3 years ago

Here's a quick and abbreviated comparison of lsof -i.

Official Spotify client on macOS, controlling librespot on another host:

UDP *:57621
TCP *:57621 (LISTEN)
TCP *:59237 (LISTEN)
TCP xxx.xxx.xxx.xxx:59242->132.242.190.35.bc.googleusercontent.com:4070 (ESTABLISHED)
UDP *:mdns
UDP *:ssdp
UDP *:62204
xxx.xxx.xxx.xxx:59247->47.224.186.35.bc.googleusercontent.com:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59309->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59246->25.224.186.35.bc.googleusercontent.com:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59265->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59280->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59310->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59273->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59263->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59264->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59266->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59267->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59268->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59276->yyy.yyy.yyy.yyy:8009 (ESTABLISHED)
xxx.xxx.xxx.xxx:59279->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59269->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59278->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59271->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59270->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59272->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59307->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59308->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59274->151.101.38.248:https (ESTABLISHED)
xxx.xxx.xxx.xxx:59275->151.101.38.248:https (ESTABLISHED)

And on the librespot host it is controlling:

TCP *:42597 (LISTEN)
UDP *:mdns 
UDP *:mdns 
TCP zzz.zzz.zzz.zzz:59558->232.65.199.104.bc.googleusercontent.com:4070 (ESTABLISHED)

Where xxx.xxx.xxx.xxx is the macOS host, yyy.yyy.yyy.yyy some other Spotify Connect-capable host, and zzz.zzz.zzz.zzz the host running librespot.

kingosticks commented 3 years ago

I've noticed two things while controlling with my Android client and stopping the librespot process:

  1. librespot remains shown as the active playing device for a while.
  2. librespot lingers in the in the "select a device" list for a while.

I think it's just the second issue you are discussing here, correct? Doesn't this sound like an mDNS problem? I've found my build using --feature with-dns-sd behaves correctly and librespot is removed from the list straight away. Have you tried that? I tried to hack about with libmdns but I got a load of tokio build problems when trying to build librespot (master) using my checkout of libmdns (master) and so I've lost interest in that. Comparing Wireshark traces of both discovery libraries would probably reveal the problem.

So potentially off topic... but for the first issue I described I found the following hack works better, but I don't understand why!

diff --git a/connect/src/spirc.rs b/connect/src/spirc.rs
index 5e3ba38..f8a71cb 100644
--- a/connect/src/spirc.rs
+++ b/connect/src/spirc.rs
@@ -544,6 +544,7 @@ impl SpircTask {
                 }
             }
             SpircCommand::Shutdown => {
+                self.notify(None, true);
-                 CommandSender::new(self, MessageType::kMessageTypeGoodbye).send();
                 self.shutdown = true;
                 self.commands.close();
kingosticks commented 3 years ago

Just to add, I did the Wireshark trace and I see when stopping librespot with dns-sd there is a goodbye message sent but not when using libmdns. libmdns seems to create a goodbye message but it doesn't make it out onto the wire. This looks pretty suspicious!

roderickvd commented 3 years ago

I'm tagging this with new-api because this might be fixed when we introduce web socket support.

kingosticks commented 3 years ago

No, it's a bug in libmdns as per the proposed fix in https://github.com/librespot-org/libmdns/pull/28. I don't believe this has anything to do with how librespot works, new-api or otherwise.

roderickvd commented 3 years ago

Ah I see, I was thrown off by @devgianlu earlier https://github.com/librespot-org/librespot/issues/676#issuecomment-805188815