Rebuild the locking mechanisms for safe interruption of an existing play session.

dbus-service.c

@@ -635,13 +635,14 @@ gboolean notify_volume_callback(ShairportSync *skeleton,
   if (((iv >= -30.0) && (iv <= 0.0)) || (iv == -144.0)) {
     debug(2, ">> setting volume to %7.4f.", iv);
 
-    lock_player();
-    if (playing_conn != NULL) {
-      player_volume(iv, playing_conn);
-      playing_conn->own_airplay_volume = iv;
-      playing_conn->own_airplay_volume_set = 1;
+    pthread_cleanup_debug_mutex_lock(&principal_conn_lock, 100000, 1);
+
+    if (principal_conn != NULL) {
+      player_volume(iv, principal_conn);
+      principal_conn->own_airplay_volume = iv;
+      principal_conn->own_airplay_volume_set = 1;
     }
-    unlock_player();
+    pthread_cleanup_pop(1); // release the principal_conn lock
     config.airplay_volume = iv;
     config.last_access_to_volume_info_time = get_absolute_time_in_ns();
   } else {
@@ -873,8 +874,6 @@ static gboolean on_handle_remote_command(ShairportSync *skeleton, GDBusMethodInv
 
 static gboolean on_handle_drop_session(ShairportSync *skeleton, GDBusMethodInvocation *invocation,
                                        __attribute__((unused)) gpointer user_data) {
-  if (playing_conn != NULL)
-    debug(1, ">> stopping current play session");
   get_play_lock(NULL, 1); // stop any current session and don't replace it
   shairport_sync_complete_drop_session(skeleton, invocation);
   return TRUE;
@@ -1071,7 +1070,8 @@ static void on_dbus_name_acquired(GDBusConnection *connection, const gchar *name
   if (config.volume_control_profile == VCP_standard)
     shairport_sync_set_volume_control_profile(SHAIRPORT_SYNC(shairportSyncSkeleton), "standard");
   else if (config.volume_control_profile == VCP_dasl_tapered)
-    shairport_sync_set_volume_control_profile(SHAIRPORT_SYNC(shairportSyncSkeleton), "dasl_tapered");
+    shairport_sync_set_volume_control_profile(SHAIRPORT_SYNC(shairportSyncSkeleton),
+                                              "dasl_tapered");
   else
     shairport_sync_set_volume_control_profile(SHAIRPORT_SYNC(shairportSyncSkeleton), "flat");
 

player.c

@@ -101,7 +101,7 @@
 
 #include "activity_monitor.h"
 
-// m<ake the first audio packet deliberately early to bias the sync error of
+// make the first audio packet deliberately early to bias the sync error of
 // the very first packet, making the error more likely to be too early
 // rather than too late. It it's too early,
 // a delay exactly compensating for it can be sent just before the
@@ -1777,20 +1777,15 @@ double suggested_volume(rtsp_conn_info *conn) {
 
 void player_thread_cleanup_handler(void *arg) {
   rtsp_conn_info *conn = (rtsp_conn_info *)arg;
-  if (pthread_mutex_trylock(&playing_conn_lock) == 0) {
 
-    pthread_cleanup_push(mutex_unlock, &playing_conn_lock);
-    if (playing_conn == conn) {
-      if (config.output->stop) {
-        debug(3, "Connection %d: Stop the output backend.", conn->connection_number);
-        config.output->stop();
-      }
-    } else {
-      debug(1, "This is not the playing conn.");
+  if ((principal_conn == conn) && (conn != NULL)) {
+    if (config.output->stop) {
+      debug(2, "Connection %d: Stop the output backend.", conn->connection_number);
+      config.output->stop();
     }
-    pthread_cleanup_pop(1); // unlock the mutex
   } else {
-    debug(1, "Can not acquire play lock.");
+    if (conn != NULL)
+      debug(1, "Connection %d: this conn is not the principal_conn.", conn->connection_number);
   }
 
   int oldState;
@@ -1900,8 +1895,6 @@ void player_thread_cleanup_handler(void *arg) {
   if (conn->stream.type == ast_apple_lossless)
     terminate_decoders(conn);
 
-  // reset_anchor_info(conn);
-  // release_play_lock(conn);
   conn->rtp_running = 0;
   pthread_setcancelstate(oldState, NULL);
   debug(2, "Connection %d: player terminated.", conn->connection_number);
@@ -3631,14 +3624,21 @@ int player_prepare_to_play(rtsp_conn_info *conn) {
 }
 
 int player_play(rtsp_conn_info *conn) {
-  pthread_t *pt = malloc(sizeof(pthread_t));
-  if (pt == NULL)
-    die("Couldn't allocate space for pthread_t");
-  conn->player_thread = pt;
-  int rc = pthread_create(pt, NULL, player_thread_func, (void *)conn);
-  if (rc)
-    debug(1, "Error creating player_thread: %s", strerror(errno));
-
+  debug(2, "Connection %d: player_play.", conn->connection_number);
+  pthread_cleanup_debug_mutex_lock(&conn->player_create_delete_mutex, 5000, 1);
+  if (conn->player_thread == NULL) {
+    pthread_t *pt = malloc(sizeof(pthread_t));
+    if (pt == NULL)
+      die("Couldn't allocate space for pthread_t");
+    int rc = pthread_create(pt, NULL, player_thread_func, (void *)conn);
+    if (rc)
+      debug(1, "Connection %d: error creating player_thread: %s", conn->connection_number,
+            strerror(errno));
+    conn->player_thread = pt; // set _after_ creation of thread
+  } else {
+    debug(1, "Connection %d: player thread already exists.", conn->connection_number);
+  }
+  pthread_cleanup_pop(1); // release the player_create_delete_mutex
 #ifdef CONFIG_METADATA
   send_ssnc_metadata('pbeg', NULL, 0, 1); // contains cancellation points
 #endif
@@ -3647,35 +3647,38 @@ int player_play(rtsp_conn_info *conn) {
 
 int player_stop(rtsp_conn_info *conn) {
   // note -- this may be called from another connection thread.
-  // int dl = debuglev;
-  // debuglev = 3;
-  debug(3, "player_stop");
-  if (conn->player_thread) {
-#ifdef CONFIG_AIRPLAY_2
-    ptp_send_control_message_string("E"); // signify play is "E"nding
-#endif
+  debug(2, "Connection %d: player_stop.", conn->connection_number);
+  int response = 0; // okay
+  pthread_cleanup_debug_mutex_lock(&conn->player_create_delete_mutex, 5000, 1);
+  pthread_t *pt = conn->player_thread;
+  if (pt) {
     debug(3, "player_thread cancel...");
-    pthread_cancel(*conn->player_thread);
+    conn->player_thread = NULL; // cleared _before_ cancelling of thread
+    pthread_cancel(*pt);
     debug(3, "player_thread join...");
-    if (pthread_join(*conn->player_thread, NULL) == -1) {
+    if (pthread_join(*pt, NULL) == -1) {
       char errorstring[1024];
       strerror_r(errno, (char *)errorstring, sizeof(errorstring));
       debug(1, "Connection %d: error %d joining player thread: \"%s\".", conn->connection_number,
             errno, (char *)errorstring);
     } else {
-      debug(3, "player_thread joined.");
+      debug(2, "Connection %d: player_stop successful.", conn->connection_number);
     }
-    free(conn->player_thread);
-    conn->player_thread = NULL;
+    free(pt);
+    response = 0; // deleted
+  } else {
+    debug(2, "Connection %d: no player thread.", conn->connection_number);
+    response = -1; // already deleted or never created...
+  }
+  pthread_cleanup_pop(1); // release the player_create_delete_mutex
+  if (response == 0) {    // if the thread was just stopped and deleted...
+#ifdef CONFIG_AIRPLAY_2
+    ptp_send_control_message_string("E"); // signify play is "E"nding
+#endif
 #ifdef CONFIG_METADATA
     send_ssnc_metadata('pend', NULL, 0, 1); // contains cancellation points
 #endif
-    // debuglev = dl;
     command_stop();
-    return 0;
-  } else {
-    debug(3, "Connection %d: player thread already deleted.", conn->connection_number);
-    // debuglev = dl;
-    return -1;
   }
+  return response;
 }

player.h

@@ -107,7 +107,8 @@ typedef enum {
   unspecified_stream_category = 0,
   ptp_stream,
   ntp_stream,
-  remote_control_stream
+  remote_control_stream,
+  classic_airplay_stream
 } airplay_stream_c; // "c" for category
 
 #ifdef CONFIG_AIRPLAY_2
@@ -221,7 +222,7 @@ typedef struct {
   int32_t last_seqno_read;
   // mutexes and condition variables
   pthread_cond_t flowcontrol;
-  pthread_mutex_t ab_mutex, flush_mutex, volume_control_mutex;
+  pthread_mutex_t ab_mutex, flush_mutex, volume_control_mutex, player_create_delete_mutex;
 
   int fix_volume;
   double own_airplay_volume;
@@ -414,7 +415,7 @@ typedef struct {
   uint64_t dac_buffer_queue_minimum_length;
 } rtsp_conn_info;
 
-extern pthread_mutex_t playing_conn_lock;
+extern pthread_mutex_t principal_conn_lock;
 extern int statistics_row; // will be reset to zero when debug level changes or statistics enabled
 
 void reset_buffer(rtsp_conn_info *conn);

rtsp.h

@@ -3,7 +3,7 @@
 
 #include "player.h"
 
-extern rtsp_conn_info *playing_conn;
+extern rtsp_conn_info *principal_conn;
 extern rtsp_conn_info **conns;
 
 void *rtsp_listen_loop(__attribute((unused)) void *arg);