When a connection termiates abruptly while is it the principal_conn, make sure it...

to NULL and cleans up the bonjour flags, if appropriate.

Simplify the TEARDOWN handlers and the thress teardown functions by incporporating
the above code in the teardown_phase_two (for AP2) and teardown (fpr AP1) functions.

It means that closing a connection will block on the principal_conn_lock, so if
you have the principal_conn_lock, closing will not complete until you release it.

Maybe we need a principal_conn_acquisition_lock for that...

diff --git a/rtsp.c b/rtsp.c
index 84ecad06b6b5422a801df57481aa55cc749c7e64..e81e398aec96b50653fc046f65c6aeb82da90bbd 100644 (file)
--- a/rtsp.c
+++ b/rtsp.c
@@ -596,6 +596,9 @@ int get_play_lock(rtsp_conn_info *conn, int allow_session_interruption) {
     rtsp_conn_info *previous_principal_conn = principal_conn;
     principal_conn = NULL; // no longer the principal conn
     pthread_cancel(previous_principal_conn->thread);
+    // the previous principal thread will block on the principal conn lock when exiting
+    // so it's important not to wait for it here, e.g. don't put in a pthread_join here.
+    // threads are garbage-collected later
     usleep(1000000);       // don't know why this delay is needed.
     principal_conn = conn; // make the conn the new principal_conn
     response = 1;          // interrupted an existing session
@@ -2705,6 +2708,23 @@ void teardown_phase_two(rtsp_conn_info *conn) {
     }
     clear_ptp_clock();
   }
+
+  // only update these things if you're (still) the principal conn
+  pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed
+  pthread_cleanup_push(rwlock_unlock, (void *)&principal_conn_lock);
+  if (principal_conn == conn) {
+    if (conn->airplay_stream_category == ptp_stream) {
+      config.airplay_statusflags &= (0xffffffff - (1 << 11)); // DeviceSupportsRelay
+      build_bonjour_strings(conn);
+      debug(2, "Connection %d: TEARDOWN mdns_update on %s.", conn->connection_number,
+            get_category_string(conn->airplay_stream_category));
+      mdns_update(NULL, secondary_txt_records);
+    }
+    principal_conn = NULL; // stop being principal_conn
+  }
+  pthread_cleanup_pop(1); // release the principal_conn lock
+  debug(2, "Connection %d: TEARDOWN %s -- close the connection complete", conn->connection_number,
+        get_category_string(conn->airplay_stream_category));
 }
 
 void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message *req,
@@ -2733,23 +2753,7 @@ void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_messag
             get_category_string(conn->airplay_stream_category));
       teardown_phase_one(conn); // try to do phase one anyway
       teardown_phase_two(conn);
-
-      // only update these things if you're (still) the principal conn
-      pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed
-      pthread_cleanup_push(rwlock_unlock, (void *)&principal_conn_lock);
-      if ((principal_conn == conn) && (conn->airplay_stream_category == ptp_stream)) {
-        config.airplay_statusflags &= (0xffffffff - (1 << 11)); // DeviceSupportsRelay
-        build_bonjour_strings(conn);
-        debug(2, "Connection %d: TEARDOWN mdns_update on %s.", conn->connection_number,
-              get_category_string(conn->airplay_stream_category));
-        mdns_update(NULL, secondary_txt_records);
-      }
-      pthread_cleanup_pop(1); // release the principal_conn lock
-      debug(2, "Connection %d: TEARDOWN %s -- close the connection complete",
-            conn->connection_number, get_category_string(conn->airplay_stream_category));
-      release_play_lock(conn);
     }
-
     plist_free(messagePlist);
     resp->respcode = 200;
   } else {
@@ -2770,40 +2774,33 @@ void teardown(rtsp_conn_info *conn) {
     free(conn->dacp_active_remote);
     conn->dacp_active_remote = NULL;
   }
-}
 
-void handle_teardown(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message *req,
-                     rtsp_message *resp) {
-  debug_log_rtsp_message(2, "TEARDOWN request", req);
-  debug(2, "Connection %d: TEARDOWN", conn->connection_number);
-  debug(3,
-        "TEARDOWN: synchronously terminating the player thread of RTSP conversation thread %d (2).",
-        conn->connection_number);
-  teardown(conn);
-
-#ifdef CONFIG_AIRPLAY_2
   // only update these things if you're (still) the principal conn
   pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed
   pthread_cleanup_push(rwlock_unlock, (void *)&principal_conn_lock);
   if (principal_conn == conn) {
+#ifdef CONFIG_AIRPLAY_2
     config.airplay_statusflags &= (0xffffffff - (1 << 11)); // DeviceSupportsRelay
     build_bonjour_strings(conn);
     mdns_update(NULL, secondary_txt_records);
+#endif
+    principal_conn = NULL; // stop being principal_conn
   }
   pthread_cleanup_pop(1); // release the principal_conn lock
-#endif
-
-  release_play_lock(conn);
+}
 
+void handle_teardown(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message *req,
+                     rtsp_message *resp) {
+  debug_log_rtsp_message(2, "TEARDOWN request", req);
+  debug(2, "Connection %d: TEARDOWN", conn->connection_number);
+  debug(3,
+        "TEARDOWN: synchronously terminating the player thread of RTSP conversation thread %d (2).",
+        conn->connection_number);
+  teardown(conn);
   resp->respcode = 200;
   msg_add_header(resp, "Connection", "close");
   debug(3, "TEARDOWN: successful termination of playing thread of RTSP conversation thread %d.",
         conn->connection_number);
-  //} else {
-  //  warn("Connection %d TEARDOWN received without having the player (no ANNOUNCE?)",
-  //       conn->connection_number);
-  //  resp->respcode = 451;
-  // }
   // debug(1,"Bogus exit for valgrind -- remember to comment it out!.");
   // exit(EXIT_SUCCESS);
 }
@@ -5163,7 +5160,7 @@ void rtsp_conversation_thread_cleanup_function(void *arg) {
     pthread_mutex_destroy(&conn->watchdog_mutex);
 
     debug(2, "Connection %d: Closed.", conn->connection_number);
-    conn->running = 0;
+    conn->running = 0; // for the garbage collector
     pthread_setcancelstate(oldState, NULL);
   }
 }