htsp: reshuffle some code to prevent unexpected async messages on shutdown

valgrind reported:
   Invalid write of size 8
      at 0x43000F: htsp_serve (htsp_server.c:2510)
      by 0x4147D2: tcp_server_start (tcp.c:447)
      by 0x412250: thread_wrapper (wrappers.c:125)
      by 0x771CB4F: start_thread (pthread_create.c:304)
      by 0x7E97E6C: clone (clone.S:112)
    Address 0x11c30ca8 is 120 bytes inside a block of size 264 free'd
      at 0x4C27D4E: free (vg_replace_malloc.c:427)
      by 0x42FF27: htsp_serve (htsp_server.c:2488)
      by 0x4147D2: tcp_server_start (tcp.c:447)
      by 0x412250: thread_wrapper (wrappers.c:125)
      by 0x771CB4F: start_thread (pthread_create.c:304)
      by 0x7E97E6C: clone (clone.S:112)

The client was removed from the async list after all connections
were destroyed, but queues are part of the connection structure,
so sporadically, an async msg was queued after the queue flush.

This code change moves the async unlink before the connection
destroy call.

diff --git a/src/htsp_server.c b/src/htsp_server.c
index cbbebe48a209fe2a60acad11198b6c5245ca0c7f..5204d5deb6e37dea77480c186aea210f874ef2cc 100644 (file)
--- a/src/htsp_server.c
+++ b/src/htsp_server.c
@@ -2374,13 +2374,9 @@ htsp_write_scheduler(void *aux)
 
   pthread_mutex_lock(&htsp->htsp_out_mutex);
 
-  while(tvheadend_running) {
+  while(htsp->htsp_writer_run) {
 
     if((hmq = TAILQ_FIRST(&htsp->htsp_active_output_queues)) == NULL) {
-      /* No active queues at all */
-      if(!htsp->htsp_writer_run)
-        break; /* Should not run anymore, bail out */
-      
       /* Nothing to be done, go to sleep */
       pthread_cond_wait(&htsp->htsp_out_cond, &htsp->htsp_out_mutex);
       continue;
@@ -2481,6 +2477,13 @@ htsp_serve(int fd, void **opaque, struct sockaddr_storage *source,
 
   pthread_mutex_lock(&global_lock);
 
+  /* no async notifications from now */
+  if(htsp.htsp_async_mode)
+    LIST_REMOVE(&htsp, htsp_async_link);
+
+  /* deregister this client */
+  LIST_REMOVE(&htsp, htsp_link);
+
   /* Beware! Closing subscriptions will invoke a lot of callbacks
      down in the streaming code. So we do this as early as possible
      to avoid any weird lockups */
@@ -2488,11 +2491,6 @@ htsp_serve(int fd, void **opaque, struct sockaddr_storage *source,
     htsp_subscription_destroy(&htsp, s);
   }
 
-  if(htsp.htsp_async_mode)
-    LIST_REMOVE(&htsp, htsp_async_link);
-
-  LIST_REMOVE(&htsp, htsp_link);
-
   pthread_mutex_unlock(&global_lock);
 
   pthread_mutex_lock(&htsp.htsp_out_mutex);
@@ -2601,6 +2599,7 @@ htsp_async_send(htsmsg_t *m, int mode)
 {
   htsp_connection_t *htsp;
 
+  lock_assert(&global_lock);
   LIST_FOREACH(htsp, &htsp_async_connections, htsp_async_link)
     if (htsp->htsp_async_mode & mode)
       htsp_send_message(htsp, htsmsg_copy(m), NULL);