Change counting to spirc and player

They can be reinstantiated, unlike the `session` which is now
intended to be constructed once.

connect/src/spirc.rs

@@ -2,6 +2,7 @@ use std::{
     convert::TryFrom,
     future::Future,
     pin::Pin,
+    sync::atomic::{AtomicUsize, Ordering},
     time::{SystemTime, UNIX_EPOCH},
 };
 
@@ -106,8 +107,12 @@ struct SpircTask {
     context_fut: BoxedFuture<Result<serde_json::Value, Error>>,
     autoplay_fut: BoxedFuture<Result<String, Error>>,
     context: Option<StationContext>,
+
+    spirc_id: usize,
 }
 
+static SPIRC_COUNTER: AtomicUsize = AtomicUsize::new(0);
+
 pub enum SpircCommand {
     Play,
     PlayPause,
@@ -263,7 +268,8 @@ impl Spirc {
         player: Player,
         mixer: Box<dyn Mixer>,
     ) -> Result<(Spirc, impl Future<Output = ()>), Error> {
-        debug!("new Spirc[{}]", session.session_id());
+        let spirc_id = SPIRC_COUNTER.fetch_add(1, Ordering::AcqRel);
+        debug!("new Spirc[{}]", spirc_id);
 
         let ident = session.device_id().to_owned();
 
@@ -368,6 +374,8 @@ impl Spirc {
             context_fut: Box::pin(future::pending()),
             autoplay_fut: Box::pin(future::pending()),
             context: None,
+
+            spirc_id,
         };
 
         if let Some(volume) = initial_volume {
@@ -1427,7 +1435,7 @@ impl SpircTask {
 
 impl Drop for SpircTask {
     fn drop(&mut self) {
-        debug!("drop Spirc[{}]", self.session.session_id());
+        debug!("drop Spirc[{}]", self.spirc_id);
     }
 }
 

core/src/session.rs

@@ -4,10 +4,7 @@ use std::{
     io,
     pin::Pin,
     process::exit,
-    sync::{
+    sync::{Arc, Weak},
-        atomic::{AtomicUsize, Ordering},
-        Arc, Weak,
-    },
     task::{Context, Poll},
     time::{SystemTime, UNIX_EPOCH},
 };
@@ -97,12 +94,8 @@ struct SessionInternal {
     cache: Option<Arc<Cache>>,
 
     handle: tokio::runtime::Handle,
-
-    session_id: usize,
 }
 
-static SESSION_COUNTER: AtomicUsize = AtomicUsize::new(0);
-
 #[derive(Clone)]
 pub struct Session(Arc<SessionInternal>);
 
@@ -110,8 +103,7 @@ impl Session {
     pub fn new(config: SessionConfig, cache: Option<Cache>) -> Self {
         let http_client = HttpClient::new(config.proxy.as_ref());
 
-        let session_id = SESSION_COUNTER.fetch_add(1, Ordering::AcqRel);
+        debug!("new Session");
-        debug!("new Session[{}]", session_id);
 
         Self(Arc::new(SessionInternal {
             config,
@@ -126,7 +118,6 @@ impl Session {
             spclient: OnceCell::new(),
             token_provider: OnceCell::new(),
             handle: tokio::runtime::Handle::current(),
-            session_id,
         }))
     }
 
@@ -218,8 +209,7 @@ impl Session {
 
     fn debug_info(&self) {
         debug!(
-            "Session[{}] strong={} weak={}",
+            "Session strong={} weak={}",
-            self.0.session_id,
             Arc::strong_count(&self.0),
             Arc::weak_count(&self.0)
         );
@@ -413,12 +403,8 @@ impl Session {
         SessionWeak(Arc::downgrade(&self.0))
     }
 
-    pub fn session_id(&self) -> usize {
-        self.0.session_id
-    }
-
     pub fn shutdown(&self) {
-        debug!("Invalidating session [{}]", self.0.session_id);
+        debug!("Invalidating session");
         self.0.data.write().invalid = true;
         self.mercury().shutdown();
         self.channel().shutdown();
@@ -445,7 +431,7 @@ impl SessionWeak {
 
 impl Drop for SessionInternal {
     fn drop(&mut self) {
-        debug!("drop Session[{}]", self.session_id);
+        debug!("drop Session");
     }
 }
 

playback/src/player.rs

@@ -7,7 +7,10 @@ use std::{
     mem,
     pin::Pin,
     process::exit,
-    sync::Arc,
+    sync::{
+        atomic::{AtomicUsize, Ordering},
+        Arc,
+    },
     task::{Context, Poll},
     thread,
     time::{Duration, Instant},
@@ -84,8 +87,12 @@ struct PlayerInternal {
     normalisation_peak: f64,
 
     auto_normalise_as_album: bool,
+
+    player_id: usize,
 }
 
+static PLAYER_COUNTER: AtomicUsize = AtomicUsize::new(0);
+
 enum PlayerCommand {
     Load {
         track_id: SpotifyId,
@@ -365,7 +372,8 @@ impl Player {
         }
 
         let handle = thread::spawn(move || {
-            debug!("new Player[{}]", session.session_id());
+            let player_id = PLAYER_COUNTER.fetch_add(1, Ordering::AcqRel);
+            debug!("new Player [{}]", player_id);
 
             let converter = Converter::new(config.ditherer);
 
@@ -388,6 +396,8 @@ impl Player {
                 normalisation_integrator: 0.0,
 
                 auto_normalise_as_album: false,
+
+                player_id,
             };
 
             // While PlayerInternal is written as a future, it still contains blocking code.
@@ -488,9 +498,8 @@ impl Drop for Player {
         debug!("Shutting down player thread ...");
         self.commands = None;
         if let Some(handle) = self.thread_handle.take() {
-            match handle.join() {
+            if let Err(e) = handle.join() {
-                Ok(_) => (),
+                error!("Player thread Error: {:?}", e);
-                Err(e) => error!("Player thread Error: {:?}", e),
             }
         }
     }
@@ -2043,7 +2052,7 @@ impl PlayerInternal {
 
 impl Drop for PlayerInternal {
     fn drop(&mut self) {
-        debug!("drop PlayerInternal[{}]", self.session.session_id());
+        debug!("drop PlayerInternal[{}]", self.player_id);
 
         let handles: Vec<thread::JoinHandle<()>> = {
             // waiting for the thread while holding the mutex would result in a deadlock