]> git.ipfire.org Git - thirdparty/shairport-sync.git/commitdiff
handle_announce() waits for playing_conn to stop if it's already shutting down
authorBelbo <belbo@gmx.de>
Mon, 7 Aug 2017 21:42:51 +0000 (23:42 +0200)
committerBelbo <belbo@gmx.de>
Tue, 31 Oct 2017 22:19:22 +0000 (23:19 +0100)
- If playing_conn is already stopping, handle_announce() waits one second for it to stop. This gives the current RTSP connection a better chance to get the player, if it has been started very quickly after the previous RTSP connection had decided to stop. iTunes can sometimes be very quick in starting a new RTSP connection after tearing down the previous one, but shairport_sync unfortunately needs at least 100ms to shut down the old connection.
- cleanup_threads() sets playing_conn to NULL if it points to the removed thread, otherwise playing_conn would be a dangling pointer, which is bad, because handle_announce() dereferences it unless it is NULL.

rtsp.c

diff --git a/rtsp.c b/rtsp.c
index 07f25bef6054aeb62c60b582dac5fa81577ac9d4..1213fa496d5f5c0b5c20e97d8c4d35de8b437ae1 100644 (file)
--- a/rtsp.c
+++ b/rtsp.c
@@ -268,6 +268,8 @@ static void cleanup_threads(void) {
             conns[i]->connection_number);
       pthread_join(conns[i]->thread, &retval);
       debug(3, "RTSP connection thread %d deleted...", conns[i]->connection_number);
+      if (conns[i] == playing_conn)
+        playing_conn = NULL;
       free(conns[i]);
       nconns--;
       if (nconns)
@@ -1395,36 +1397,37 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag
   if (pthread_mutex_trylock(&play_lock) == 0) {
     have_the_player = 1;
   } else {
-    if (playing_conn) {
-      debug(1, "RTSP Conversation thread %d already playing when asked by thread %d.",
-            playing_conn->connection_number, conn->connection_number);
-    } else {
-      debug(1, "play_lock locked but no playing_conn.");
-    }
-    if (config.allow_session_interruption == 1) {
+    int should_wait = 0;
+
+    if (! playing_conn)
+      die("Non existent playing_conn with play_lock enabled.");
+    debug(1, "RTSP Conversation thread %d already playing when asked by thread %d.",
+          playing_conn->connection_number, conn->connection_number);
+    if (playing_conn->stop) {
+      debug(1,"Playing connection is already shutting down; waiting for it...");
+      should_wait = 1;
+    } else if (config.allow_session_interruption == 1) {
       // some other thread has the player ... ask it to relinquish the thread
-      if (playing_conn) {
-        debug(1, "ANNOUNCE: playing connection %d being interrupted by connection %d.",
-              playing_conn->connection_number, conn->connection_number);
-        if (playing_conn == conn) {
-          debug(1, "ANNOUNCE asking to stop itself.");
-        } else {
-          playing_conn->stop = 1;
-          memory_barrier();
-          pthread_kill(playing_conn->thread, SIGUSR1);
-          usleep(
-              1000000); // here, it is possible for other connections to come in and nab the player.
-        }
+      debug(1, "ANNOUNCE: playing connection %d being interrupted by connection %d.",
+            playing_conn->connection_number, conn->connection_number);
+      if (playing_conn == conn) {
+        debug(1, "ANNOUNCE asking to stop itself.");
       } else {
-        die("Non existent the_playing_conn with play_lock enabled.");
+        playing_conn->stop = 1;
+        memory_barrier();
+        pthread_kill(playing_conn->thread, SIGUSR1);
+        should_wait = 1;
       }
+    }
+
+    if (should_wait) {
+      usleep(1000000); // here, it is possible for other connections to come in and nab the player.
       debug(1, "Try to get the player now");
-      // pthread_mutex_lock(&play_lock);
-      if (pthread_mutex_trylock(&play_lock) == 0)
-        have_the_player = 1;
-      else
-        debug(1, "ANNOUNCE failed to get the player");
     }
+    if (pthread_mutex_trylock(&play_lock) == 0)
+      have_the_player = 1;
+    else
+      debug(1,"ANNOUNCE failed to get the player");
   }
 
   if (have_the_player) {