From 22f060842165e498e7905ebaf14296d19aa65a02 Mon Sep 17 00:00:00 2001 From: Jaroslav Kysela Date: Fri, 3 Oct 2014 11:35:06 +0200 Subject: [PATCH] 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. --- src/htsp_server.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/htsp_server.c b/src/htsp_server.c index cbbebe48..5204d5de 100644 --- 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);