]> git.ipfire.org Git - thirdparty/shairport-sync.git/commitdiff
Fix a bug that sometimes caused a crash when a service name was specified in the...
authorMike Brady <mikebrady@eircom.net>
Sun, 21 Jul 2019 07:06:37 +0000 (09:06 +0200)
committerMike Brady <mikebrady@eircom.net>
Sun, 21 Jul 2019 07:06:37 +0000 (09:06 +0200)
Allow the rtsp_listen_loop to exit if the RTSP port (usually port 5000) is not available. This allows the system to exit cleanly rather than abort.

rtsp.c
shairport.c

diff --git a/rtsp.c b/rtsp.c
index 20fc6b8b58de4f1c499e8d3939b4e1d0e8eb6319..4a72250afb0f79009fed3914fcf7c0279c42caf0 100644 (file)
--- a/rtsp.c
+++ b/rtsp.c
@@ -2572,7 +2572,7 @@ void rtsp_listen_loop(void) {
       } else
 #endif
         family = "IPv4";
-      debug(1, "Unable to listen on %s port %d. The error is: \"%s\".", family, config.port,
+      debug(1, "unable to listen on %s port %d. The error is: \"%s\".", family, config.port,
             strerror(errno));
       continue;
     }
@@ -2585,127 +2585,127 @@ void rtsp_listen_loop(void) {
 
   freeaddrinfo(info);
 
-  if (!nsock)
-    die("Could not establish a service on port %d -- program terminating. Is another instance of "
-        "Shairport Sync running on this device?",
-        config.port);
-
-  int maxfd = -1;
-  fd_set fds;
-  FD_ZERO(&fds);
-  for (i = 0; i < nsock; i++) {
-    if (sockfd[i] > maxfd)
-      maxfd = sockfd[i];
-  }
-
-  mdns_register();
-
-  pthread_setcancelstate(oldState, NULL);
-  int acceptfd;
-  struct timeval tv;
-  pthread_cleanup_push(rtsp_listen_loop_cleanup_handler, (void *)sockfd);
-  do {
-    pthread_testcancel();
-    tv.tv_sec = 60;
-    tv.tv_usec = 0;
+  if (nsock) {
+    int maxfd = -1;
+    fd_set fds;
+    FD_ZERO(&fds);
+    for (i = 0; i < nsock; i++) {
+      if (sockfd[i] > maxfd)
+        maxfd = sockfd[i];
+    }
 
-    for (i = 0; i < nsock; i++)
-      FD_SET(sockfd[i], &fds);
+    mdns_register();
 
-    ret = select(maxfd + 1, &fds, 0, 0, &tv);
-    if (ret < 0) {
-      if (errno == EINTR)
-        continue;
-      break;
-    }
+    pthread_setcancelstate(oldState, NULL);
+    int acceptfd;
+    struct timeval tv;
+    pthread_cleanup_push(rtsp_listen_loop_cleanup_handler, (void *)sockfd);
+    do {
+      pthread_testcancel();
+      tv.tv_sec = 60;
+      tv.tv_usec = 0;
 
-    cleanup_threads();
+      for (i = 0; i < nsock; i++)
+        FD_SET(sockfd[i], &fds);
 
-    acceptfd = -1;
-    for (i = 0; i < nsock; i++) {
-      if (FD_ISSET(sockfd[i], &fds)) {
-        acceptfd = sockfd[i];
+      ret = select(maxfd + 1, &fds, 0, 0, &tv);
+      if (ret < 0) {
+        if (errno == EINTR)
+          continue;
         break;
       }
-    }
-    if (acceptfd < 0) // timeout
-      continue;
 
-    rtsp_conn_info *conn = malloc(sizeof(rtsp_conn_info));
-    if (conn == 0)
-      die("Couldn't allocate memory for an rtsp_conn_info record.");
-    memset(conn, 0, sizeof(rtsp_conn_info));
-    conn->connection_number = RTSP_connection_index++;
-    socklen_t slen = sizeof(conn->remote);
-
-    conn->fd = accept(acceptfd, (struct sockaddr *)&conn->remote, &slen);
-    if (conn->fd < 0) {
-      debug(1, "Connection %d: New connection on port %d not accepted:", conn->connection_number,
-            config.port);
-      perror("failed to accept connection");
-      free(conn);
-    } else {
-      SOCKADDR *local_info = (SOCKADDR *)&conn->local;
-      socklen_t size_of_reply = sizeof(*local_info);
-      memset(local_info, 0, sizeof(SOCKADDR));
-      if (getsockname(conn->fd, (struct sockaddr *)local_info, &size_of_reply) == 0) {
-
-        // IPv4:
-        if (local_info->SAFAMILY == AF_INET) {
-          char ip4[INET_ADDRSTRLEN];        // space to hold the IPv4 string
-          char remote_ip4[INET_ADDRSTRLEN]; // space to hold the IPv4 string
-          struct sockaddr_in *sa = (struct sockaddr_in *)local_info;
-          inet_ntop(AF_INET, &(sa->sin_addr), ip4, INET_ADDRSTRLEN);
-          unsigned short int tport = ntohs(sa->sin_port);
-          sa = (struct sockaddr_in *)&conn->remote;
-          inet_ntop(AF_INET, &(sa->sin_addr), remote_ip4, INET_ADDRSTRLEN);
-          unsigned short int rport = ntohs(sa->sin_port);
-          debug(2, "Connection %d: new connection from %s:%u to self at %s:%u.",
-                conn->connection_number, remote_ip4, rport, ip4, tport);
-        }
-#ifdef AF_INET6
-        if (local_info->SAFAMILY == AF_INET6) {
-          // IPv6:
-
-          char ip6[INET6_ADDRSTRLEN];        // space to hold the IPv6 string
-          char remote_ip6[INET6_ADDRSTRLEN]; // space to hold the IPv6 string
-          struct sockaddr_in6 *sa6 =
-              (struct sockaddr_in6 *)local_info; // pretend this is loaded with something
-          inet_ntop(AF_INET6, &(sa6->sin6_addr), ip6, INET6_ADDRSTRLEN);
-          u_int16_t tport = ntohs(sa6->sin6_port);
-
-          sa6 = (struct sockaddr_in6 *)&conn->remote; // pretend this is loaded with something
-          inet_ntop(AF_INET6, &(sa6->sin6_addr), remote_ip6, INET6_ADDRSTRLEN);
-          u_int16_t rport = ntohs(sa6->sin6_port);
-          debug(2, "Connection %d: new connection from [%s]:%u to self at [%s]:%u.",
-                conn->connection_number, remote_ip6, rport, ip6, tport);
+      cleanup_threads();
+
+      acceptfd = -1;
+      for (i = 0; i < nsock; i++) {
+        if (FD_ISSET(sockfd[i], &fds)) {
+          acceptfd = sockfd[i];
+          break;
         }
-#endif
+      }
+      if (acceptfd < 0) // timeout
+        continue;
 
+      rtsp_conn_info *conn = malloc(sizeof(rtsp_conn_info));
+      if (conn == 0)
+        die("Couldn't allocate memory for an rtsp_conn_info record.");
+      memset(conn, 0, sizeof(rtsp_conn_info));
+      conn->connection_number = RTSP_connection_index++;
+      socklen_t slen = sizeof(conn->remote);
+
+      conn->fd = accept(acceptfd, (struct sockaddr *)&conn->remote, &slen);
+      if (conn->fd < 0) {
+        debug(1, "Connection %d: New connection on port %d not accepted:", conn->connection_number,
+              config.port);
+        perror("failed to accept connection");
+        free(conn);
       } else {
-        debug(1, "Error figuring out Shairport Sync's own IP number.");
-      }
-      //      usleep(500000);
-      //      pthread_t rtsp_conversation_thread;
-      //      conn->thread = rtsp_conversation_thread;
-      //      conn->stop = 0; // record's memory has been zeroed
-      //      conn->authorized = 0; // record's memory has been zeroed
-      // fcntl(conn->fd, F_SETFL, O_NONBLOCK);
-
-      ret = pthread_create(&conn->thread, NULL, rtsp_conversation_thread_func,
-                           conn); // also acts as a memory barrier
-      if (ret) {
-        char errorstring[1024];
-        strerror_r(ret, (char *)errorstring, sizeof(errorstring));
-        die("Connection %d: cannot create an RTSP conversation thread. Error %d: \"%s\".",
-            conn->connection_number, ret, (char *)errorstring);
-      }
-      debug(3, "Successfully created RTSP receiver thread %d.", conn->connection_number);
-      conn->running = 1; // this must happen before the thread is tracked
-      track_thread(conn);
-    }
-  } while (1);
+        SOCKADDR *local_info = (SOCKADDR *)&conn->local;
+        socklen_t size_of_reply = sizeof(*local_info);
+        memset(local_info, 0, sizeof(SOCKADDR));
+        if (getsockname(conn->fd, (struct sockaddr *)local_info, &size_of_reply) == 0) {
+
+          // IPv4:
+          if (local_info->SAFAMILY == AF_INET) {
+            char ip4[INET_ADDRSTRLEN];        // space to hold the IPv4 string
+            char remote_ip4[INET_ADDRSTRLEN]; // space to hold the IPv4 string
+            struct sockaddr_in *sa = (struct sockaddr_in *)local_info;
+            inet_ntop(AF_INET, &(sa->sin_addr), ip4, INET_ADDRSTRLEN);
+            unsigned short int tport = ntohs(sa->sin_port);
+            sa = (struct sockaddr_in *)&conn->remote;
+            inet_ntop(AF_INET, &(sa->sin_addr), remote_ip4, INET_ADDRSTRLEN);
+            unsigned short int rport = ntohs(sa->sin_port);
+            debug(2, "Connection %d: new connection from %s:%u to self at %s:%u.",
+                  conn->connection_number, remote_ip4, rport, ip4, tport);
+          }
+  #ifdef AF_INET6
+          if (local_info->SAFAMILY == AF_INET6) {
+            // IPv6:
+
+            char ip6[INET6_ADDRSTRLEN];        // space to hold the IPv6 string
+            char remote_ip6[INET6_ADDRSTRLEN]; // space to hold the IPv6 string
+            struct sockaddr_in6 *sa6 =
+                (struct sockaddr_in6 *)local_info; // pretend this is loaded with something
+            inet_ntop(AF_INET6, &(sa6->sin6_addr), ip6, INET6_ADDRSTRLEN);
+            u_int16_t tport = ntohs(sa6->sin6_port);
+
+            sa6 = (struct sockaddr_in6 *)&conn->remote; // pretend this is loaded with something
+            inet_ntop(AF_INET6, &(sa6->sin6_addr), remote_ip6, INET6_ADDRSTRLEN);
+            u_int16_t rport = ntohs(sa6->sin6_port);
+            debug(2, "Connection %d: new connection from [%s]:%u to self at [%s]:%u.",
+                  conn->connection_number, remote_ip6, rport, ip6, tport);
+          }
+  #endif
 
-  pthread_cleanup_pop(1); // should never happen
-  debug(1, "Oops -- fell out of the RTSP select loop");
+        } else {
+          debug(1, "Error figuring out Shairport Sync's own IP number.");
+        }
+        //      usleep(500000);
+        //      pthread_t rtsp_conversation_thread;
+        //      conn->thread = rtsp_conversation_thread;
+        //      conn->stop = 0; // record's memory has been zeroed
+        //      conn->authorized = 0; // record's memory has been zeroed
+        // fcntl(conn->fd, F_SETFL, O_NONBLOCK);
+
+        ret = pthread_create(&conn->thread, NULL, rtsp_conversation_thread_func,
+                             conn); // also acts as a memory barrier
+        if (ret) {
+          char errorstring[1024];
+          strerror_r(ret, (char *)errorstring, sizeof(errorstring));
+          die("Connection %d: cannot create an RTSP conversation thread. Error %d: \"%s\".",
+              conn->connection_number, ret, (char *)errorstring);
+        }
+        debug(3, "Successfully created RTSP receiver thread %d.", conn->connection_number);
+        conn->running = 1; // this must happen before the thread is tracked
+        track_thread(conn);
+      }
+    } while (1);
+    pthread_cleanup_pop(1); // should never happen
+  } else {
+    warn("could not establish a service on port %d -- program terminating. Is another instance of "
+        "Shairport Sync running on this device?",
+        config.port);
+  }
+  // debug(1, "Oops -- fell out of the RTSP select loop");
 }
index 499b4c0b897644f1ad608c4add4b32cc28cf8a3d..b301aecf812dbb73e5a7d16921929a99a4ac46c4 100644 (file)
@@ -200,11 +200,17 @@ void *soxr_time_check(__attribute__((unused)) void *arg) {
 
     size_t odone;
 
+    //int oldState;
+    //pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); // make this un-cancellable
+
     soxr_oneshot(buffer_length, buffer_length + 1, 2,  // Rates and # of chans.
                  inbuffer, buffer_length, NULL,        // Input.
                  outbuffer, buffer_length + 1, &odone, // Output.
                  &io_spec,                             // Input, output and transfer spec.
                  NULL, NULL);                          // Default configuration.
+    //pthread_setcancelstate(oldState, NULL);
+    
+    pthread_testcancel();
 
     io_spec.itype = SOXR_INT32_I;
     io_spec.otype = SOXR_INT32_I;
@@ -212,11 +218,13 @@ void *soxr_time_check(__attribute__((unused)) void *arg) {
     io_spec.e = NULL;
     io_spec.flags = 0;
 
+    //pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); // make this un-cancellable
     soxr_oneshot(buffer_length, buffer_length - 1, 2,  // Rates and # of chans.
                  inbuffer, buffer_length, NULL,        // Input.
                  outbuffer, buffer_length - 1, &odone, // Output.
                  &io_spec,                             // Input, output and transfer spec.
                  NULL, NULL);                          // Default configuration.
+   //pthread_setcancelstate(oldState, NULL);
   }
 
   double soxr_execution_time_us =
@@ -1162,25 +1170,27 @@ int parse_options(int argc, char **argv) {
   if (tdebuglev != 0)
     debuglev = tdebuglev;
 
-  /* if the Service Name wasn't specified, do it now */
-
-  if (raw_service_name == NULL)
-    raw_service_name = strdup("%H");
-
   // now, do the substitutions in the service name
   char hostname[100];
   gethostname(hostname, 100);
-  char *i1 = str_replace(raw_service_name, "%h", hostname);
-  if (raw_service_name) {
-    free(raw_service_name);
-    raw_service_name = NULL;
-  }
+
+  
+  
+  char *i0;
+  if (raw_service_name == NULL)
+    i0 = strdup("%H"); // this is the default it the Service Name wasn't specified
+  else
+    i0 = strdup(raw_service_name);
+  // here, do the substitutions for %h, %H, %v and %V
+  char *i1 = str_replace(i0, "%h", hostname);
   if ((hostname[0] >= 'a') && (hostname[0] <= 'z'))
     hostname[0] = hostname[0] - 0x20; // convert a lowercase first letter into a capital letter
   char *i2 = str_replace(i1, "%H", hostname);
   char *i3 = str_replace(i2, "%v", PACKAGE_VERSION);
   char *vs = get_version_string();
-  config.service_name = str_replace(i3, "%V", vs);
+  config.service_name = str_replace(i3, "%V", vs); // service name complete
+  free(i0);
   free(i1);
   free(i2);
   free(i3);
@@ -1277,8 +1287,8 @@ const char *pid_file_proc(void) {
 #endif
 
 void main_cleanup_handler(__attribute__((unused)) void *arg) {
-  // it doesn't look like this is called when the main function is cancelled eith a pthread cancel.
-  debug(2, "main cleanup handler called.");
+  // it doesn't look like this is called when the main function is cancelled with a pthread cancel.
+  debug(1, "main cleanup handler called.");
 #ifdef CONFIG_MQTT
   if (config.mqtt_enabled) {
     // terminate_mqtt();
@@ -1320,6 +1330,11 @@ void main_cleanup_handler(__attribute__((unused)) void *arg) {
     config.output->deinit();
   }
 
+#ifdef CONFIG_SOXR
+  // be careful -- not sure if the thread can be cancelled cleanly, so wait for it to shut down
+  pthread_join(soxr_time_check_thread, NULL);
+#endif
+
 #ifdef CONFIG_LIBDAEMON
   // only do this if you are the daemon
   if (pid == 0) {
@@ -1334,7 +1349,7 @@ void main_cleanup_handler(__attribute__((unused)) void *arg) {
 }
 
 void exit_function() {
-  debug(2, "exit function called...");
+  debug(1, "exit function called...");
   main_cleanup_handler(NULL);
   if (conns)
     free(conns); // make sure the connections have been deleted first
@@ -1643,7 +1658,7 @@ int main(int argc, char **argv) {
   }
   config.output->init(argc - audio_arg, argv + audio_arg);
 
-  pthread_cleanup_push(main_cleanup_handler, NULL);
+  // pthread_cleanup_push(main_cleanup_handler, NULL);
 
   // daemon_log(LOG_NOTICE, "startup");
 
@@ -1854,15 +1869,6 @@ int main(int argc, char **argv) {
 #endif
 
   activity_monitor_start();
-
-  // debug(1, "Successful Startup");
   rtsp_listen_loop();
-
-  // should not reach this...
-  // daemon_log(LOG_NOTICE, "Unexpected exit...");
-  // daemon_retval_send(0);
-  // daemon_pid_file_remove();
-  pthread_cleanup_pop(1);
-  debug(1, "Odd exit point");
-  pthread_exit(NULL);
+  return 0;
 }