Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
htsp: reshuffle some code to prevent unexpected async messages on shu…
…tdown

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.
  • Loading branch information
perexg committed Oct 3, 2014
1 parent 604ff92 commit 22f0608
Showing 1 changed file with 9 additions and 10 deletions.
19 changes: 9 additions & 10 deletions src/htsp_server.c
Expand Up @@ -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;
Expand Down Expand Up @@ -2481,18 +2477,20 @@ 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 */
while((s = LIST_FIRST(&htsp.htsp_subscriptions)) != NULL) {
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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 22f0608

Please sign in to comment.