From f1d450343135434d19296f3b728cb68f2f19698a Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Fri, 20 Jul 2018 13:50:38 +0100 Subject: Try to add proper cancellation code to all threads -- not complete, buggy. --- dacp.c | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) (limited to 'dacp.c') diff --git a/dacp.c b/dacp.c index 53b7661..81a6f4d 100644 --- a/dacp.c +++ b/dacp.c @@ -110,7 +110,10 @@ static void response_code(void *opaque, int code) { } static const struct http_funcs responseFuncs = { - response_realloc, response_body, response_header, response_code, + response_realloc, + response_body, + response_header, + response_code, }; // static pthread_mutex_t dacp_conversation_lock = PTHREAD_MUTEX_INITIALIZER; @@ -261,8 +264,9 @@ int dacp_send_command(const char *command, char **body, ssize_t *bodysize) { // debug(1,"Sent command\"%s\" with a response body of size %d.",command,response.size); // debug(1,"dacp_conversation_lock released."); } else { - debug(3, "Could not acquire a lock on the dacp transmit/receive section when attempting to " - "send the command \"%s\". Possible timeout?", + debug(3, + "Could not acquire a lock on the dacp transmit/receive section when attempting to " + "send the command \"%s\". Possible timeout?", command); response.code = 494; // This client is already busy } @@ -384,6 +388,12 @@ void dacp_monitor_port_update_callback(char *dacp_id, uint16_t port) { pthread_cond_signal(&dacp_server_information_cv); pthread_mutex_unlock(&dacp_server_information_lock); } + +void dacp_monitor_thread_code_cleanup(__attribute__((unused)) void *arg) { + debug(1, "dacp_monitor_thread_code_cleanup called."); + pthread_mutex_unlock(&dacp_server_information_lock); +} + void *dacp_monitor_thread_code(__attribute__((unused)) void *na) { int scan_index = 0; // char server_reply[10000]; @@ -397,14 +407,18 @@ void *dacp_monitor_thread_code(__attribute__((unused)) void *na) { sps_pthread_mutex_timedlock( &dacp_server_information_lock, 500000, "dacp_monitor_thread_code couldn't get DACP server information lock in 0.5 second!.", 2); + int32_t the_volume; + + pthread_cleanup_push(dacp_monitor_thread_code_cleanup, NULL); if (dacp_server.scan_enable == 0) { metadata_hub_modify_prolog(); int ch = (metadata_store.dacp_server_active != 0) || (metadata_store.advanced_dacp_server_active != 0); metadata_store.dacp_server_active = 0; metadata_store.advanced_dacp_server_active = 0; - debug(2, "setting dacp_server_active and advanced_dacp_server_active to 0 with an update " - "flag value of %d", + debug(2, + "setting dacp_server_active and advanced_dacp_server_active to 0 with an update " + "flag value of %d", ch); metadata_hub_modify_epilog(ch); while (dacp_server.scan_enable == 0) { @@ -416,7 +430,6 @@ void *dacp_monitor_thread_code(__attribute__((unused)) void *na) { idle_scan_count = 0; } scan_index++; - int32_t the_volume; result = dacp_get_volume(&the_volume); // just want the http code if ((result == 496) || (result == 403) || (result == 501)) { @@ -438,7 +451,9 @@ void *dacp_monitor_thread_code(__attribute__((unused)) void *na) { debug(1, "DACP server status scanning stopped."); dacp_server.scan_enable = 0; } - pthread_mutex_unlock(&dacp_server_information_lock); + pthread_cleanup_pop(1); + + // pthread_mutex_unlock(&dacp_server_information_lock); // debug(1, "DACP Server ID \"%u\" at \"%s:%u\", scan %d.", dacp_server.active_remote_id, // dacp_server.ip_string, dacp_server.port, scan_index); @@ -764,7 +779,7 @@ void *dacp_monitor_thread_code(__attribute__((unused)) void *na) { sleep(config.scan_interval_when_inactive); } } - debug(1, "DACP monitor thread exiting."); + debug(1, "DACP monitor thread exiting -- should never happen."); pthread_exit(NULL); } @@ -816,6 +831,14 @@ void dacp_monitor_start() { pthread_create(&dacp_monitor_thread, NULL, dacp_monitor_thread_code, NULL); } +void dacp_monitor_stop() { + debug(1, "dacp_monitor_stop"); + pthread_cancel(dacp_monitor_thread); + pthread_join(dacp_monitor_thread, NULL); + pthread_mutex_destroy(&dacp_server_information_lock); + pthread_mutex_destroy(&dacp_conversation_lock); +} + uint32_t dacp_tlv_crawl(char **p, int32_t *length) { char typecode[5]; memcpy(typecode, *p, 4); -- cgit v1.2.3 From 2eaaa9cdb3fdd25ba96f165fdf68c813cfacf25f Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sat, 21 Jul 2018 15:35:07 +0100 Subject: Fix a bug where the dacp notification handlers are called --- dacp.c | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) (limited to 'dacp.c') diff --git a/dacp.c b/dacp.c index 81a6f4d..667ac19 100644 --- a/dacp.c +++ b/dacp.c @@ -294,12 +294,10 @@ void relinquish_dacp_server_information(rtsp_conn_info *conn) { // as the conn's connection number // this is to signify that the player has stopped, but only if another thread (with a different // index) hasn't already taken over the dacp service - sps_pthread_mutex_timedlock( - &dacp_server_information_lock, 500000, - "set_dacp_server_information couldn't get DACP server information lock in 0.5 second!.", 2); + debug_mutex_lock(&dacp_server_information_lock, 500000, 2); if (dacp_server.players_connection_thread_index == conn->connection_number) dacp_server.players_connection_thread_index = 0; - pthread_mutex_unlock(&dacp_server_information_lock); + debug_mutex_unlock(&dacp_server_information_lock, 3); } // this will be running on the thread of its caller, not of the conversation thread... @@ -309,9 +307,7 @@ void relinquish_dacp_server_information(rtsp_conn_info *conn) { // Thus, we can keep the DACP port that might have previously been discovered void set_dacp_server_information(rtsp_conn_info *conn) { // debug(1, "set_dacp_server_information"); - sps_pthread_mutex_timedlock( - &dacp_server_information_lock, 500000, - "set_dacp_server_information couldn't get DACP server information lock in 0.5 second!.", 2); + debug_mutex_lock(&dacp_server_information_lock, 500000, 2); dacp_server.players_connection_thread_index = conn->connection_number; if ((conn->dacp_id == NULL) || (strcmp(conn->dacp_id, dacp_server.dacp_id) != 0)) { @@ -360,23 +356,20 @@ void set_dacp_server_information(rtsp_conn_info *conn) { debug(2, "set_dacp_server_information set active-remote id to %" PRIu32 ".", dacp_server.active_remote_id); pthread_cond_signal(&dacp_server_information_cv); - pthread_mutex_unlock(&dacp_server_information_lock); + debug_mutex_unlock(&dacp_server_information_lock, 3); } void dacp_monitor_port_update_callback(char *dacp_id, uint16_t port) { debug(2, "dacp_monitor_port_update_callback with Remote ID \"%s\" and port number %d.", dacp_id, port); - sps_pthread_mutex_timedlock( - &dacp_server_information_lock, 500000, - "dacp_monitor_port_update_callback couldn't get DACP server information lock in 0.5 second!.", - 2); + debug_mutex_lock(&dacp_server_information_lock, 500000, 2); if (strcmp(dacp_id, dacp_server.dacp_id) == 0) { dacp_server.port = port; if (port == 0) dacp_server.scan_enable = 0; else { dacp_server.scan_enable = 1; - debug(2, "dacp_monitor_port_update_callback enables scan"); + // debug(2, "dacp_monitor_port_update_callback enables scan"); } // metadata_hub_modify_prolog(); // int ch = metadata_store.dacp_server_active != dacp_server.scan_enable; @@ -386,11 +379,11 @@ void dacp_monitor_port_update_callback(char *dacp_id, uint16_t port) { debug(1, "dacp port monitor reporting on an out-of-use remote."); } pthread_cond_signal(&dacp_server_information_cv); - pthread_mutex_unlock(&dacp_server_information_lock); + debug_mutex_unlock(&dacp_server_information_lock, 3); } void dacp_monitor_thread_code_cleanup(__attribute__((unused)) void *arg) { - debug(1, "dacp_monitor_thread_code_cleanup called."); + // debug(1, "dacp_monitor_thread_code_cleanup called."); pthread_mutex_unlock(&dacp_server_information_lock); } -- cgit v1.2.3 From 5f750ba9b727c384b992711a95a32fe0a48dcecd Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sun, 22 Jul 2018 16:12:52 +0100 Subject: Fix a few memory leaks. --- dacp.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 52 insertions(+), 9 deletions(-) (limited to 'dacp.c') diff --git a/dacp.c b/dacp.c index 667ac19..80ccb2e 100644 --- a/dacp.c +++ b/dacp.c @@ -122,6 +122,35 @@ static pthread_mutex_t dacp_conversation_lock; static pthread_mutex_t dacp_server_information_lock; static pthread_cond_t dacp_server_information_cv = PTHREAD_COND_INITIALIZER; +void addrinfo_cleanup(void *arg) { + debug(1, "addrinfo cleanup called."); + struct addrinfo **info = (struct addrinfo **)arg; + freeaddrinfo(*info); +} + +void mutex_lock_cleanup(void *arg) { + debug(1, "mutex lock cleanup called."); + pthread_mutex_t *m = (pthread_mutex_t *)arg; + pthread_mutex_unlock(m); +} + +void connect_cleanup(void *arg) { + debug(1, "connect cleanup called."); + int *fd = (int *)arg; + close(*fd); +} + +void http_cleanup(void *arg) { + debug(1, "http cleanup called."); + struct http_roundtripper *rt = (struct http_roundtripper *)arg; + http_free(rt); +} + +void malloc_cleanup(void *arg) { + debug(1, "malloc cleanup called."); + free(arg); +} + int dacp_send_command(const char *command, char **body, ssize_t *bodysize) { // will malloc space for the body or set it to NULL -- the caller should free it. @@ -166,12 +195,13 @@ int dacp_send_command(const char *command, char **body, ssize_t *bodysize) { // debug(1,"Error %d \"%s\" at getaddrinfo.",ires,gai_strerror(ires)); response.code = 498; // Bad Address information for the DACP server } else { - + pthread_cleanup_push(addrinfo_cleanup, (void *)&res); // only do this one at a time -- not sure it is necessary, but better safe than sorry int mutex_reply = sps_pthread_mutex_timedlock(&dacp_conversation_lock, 2000000, command, 1); // int mutex_reply = pthread_mutex_lock(&dacp_conversation_lock); if (mutex_reply == 0) { + pthread_cleanup_push(mutex_lock_cleanup, (void *)&dacp_conversation_lock); // debug(1,"dacp_conversation_lock acquired for command \"%s\".",command); // make a socket: @@ -195,6 +225,7 @@ int dacp_send_command(const char *command, char **body, ssize_t *bodysize) { debug(3, "DACP connect failed with errno %d.", errno); response.code = 496; // Can't connect to the DACP server } else { + pthread_cleanup_push(connect_cleanup, (void *)&sockfd); // debug(1,"DACP connect succeeded."); snprintf(message, sizeof(message), @@ -213,9 +244,11 @@ int dacp_send_command(const char *command, char **body, ssize_t *bodysize) { response.body = malloc(2048); // it can resize this if necessary response.malloced_size = 2048; + pthread_cleanup_push(malloc_cleanup, response.body); struct http_roundtripper rt; http_init(&rt, responseFuncs, &response); + pthread_cleanup_push(http_cleanup, &rt); int needmore = 1; int looperror = 0; @@ -254,13 +287,18 @@ int dacp_send_command(const char *command, char **body, ssize_t *bodysize) { response.size = 0; } // debug(1,"Size of response body is %d",response.size); - http_free(&rt); + pthread_cleanup_pop(1); // this should call http_cleanup + // http_free(&rt); + pthread_cleanup_pop( + 0); // this should *not* free the malloced buffer -- just pop the malloc cleanup } + pthread_cleanup_pop(1); // this should close the socket + // close(sockfd); + // debug(1,"DACP socket closed."); } - close(sockfd); - // debug(1,"DACP socket closed."); } - pthread_mutex_unlock(&dacp_conversation_lock); + pthread_cleanup_pop(1); // this should unlock the dacp_conversation_lock); + // pthread_mutex_unlock(&dacp_conversation_lock); // debug(1,"Sent command\"%s\" with a response body of size %d.",command,response.size); // debug(1,"dacp_conversation_lock released."); } else { @@ -270,6 +308,8 @@ int dacp_send_command(const char *command, char **body, ssize_t *bodysize) { command); response.code = 494; // This client is already busy } + pthread_cleanup_pop(1); // this should free the addrinfo + // freeaddrinfo(res); } *body = response.body; *bodysize = response.size; @@ -930,12 +970,13 @@ int dacp_get_speaker_list(dacp_spkr_stuff *speaker_info, int max_size_of_array, sp -= item_size; le -= 8; speaker_index++; - if (speaker_index == max_size_of_array) + if (speaker_index == max_size_of_array) { return 413; // Payload Too Large -- too many speakers + } speaker_info[speaker_index].active = 0; speaker_info[speaker_index].speaker_number = 0; speaker_info[speaker_index].volume = 0; - speaker_info[speaker_index].name = NULL; + speaker_info[speaker_index].name[0] = '\0'; } else { le -= item_size + 8; char *t; @@ -945,8 +986,10 @@ int dacp_get_speaker_list(dacp_spkr_stuff *speaker_info, int max_size_of_array, switch (type) { case 'minm': t = sp - item_size; - speaker_info[speaker_index].name = strndup(t, item_size); - // debug(1," \"%s\"",speaker_info[speaker_index].name); + strncpy((char *)&speaker_info[speaker_index].name, t, + sizeof(speaker_info[speaker_index].name)); + speaker_info[speaker_index].name[sizeof(speaker_info[speaker_index].name) - 1] = + '\0'; // just in case break; case 'cmvo': t = sp - item_size; -- cgit v1.2.3 From 703ab8e5562d57db2e1bb08c357804627b3eea53 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sun, 22 Jul 2018 16:13:49 +0100 Subject: Quieten a few debug messages --- dacp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'dacp.c') diff --git a/dacp.c b/dacp.c index 80ccb2e..090bb52 100644 --- a/dacp.c +++ b/dacp.c @@ -123,31 +123,31 @@ static pthread_mutex_t dacp_server_information_lock; static pthread_cond_t dacp_server_information_cv = PTHREAD_COND_INITIALIZER; void addrinfo_cleanup(void *arg) { - debug(1, "addrinfo cleanup called."); + // debug(1, "addrinfo cleanup called."); struct addrinfo **info = (struct addrinfo **)arg; freeaddrinfo(*info); } void mutex_lock_cleanup(void *arg) { - debug(1, "mutex lock cleanup called."); + // debug(1, "mutex lock cleanup called."); pthread_mutex_t *m = (pthread_mutex_t *)arg; pthread_mutex_unlock(m); } void connect_cleanup(void *arg) { - debug(1, "connect cleanup called."); + // debug(1, "connect cleanup called."); int *fd = (int *)arg; close(*fd); } void http_cleanup(void *arg) { - debug(1, "http cleanup called."); + // debug(1, "http cleanup called."); struct http_roundtripper *rt = (struct http_roundtripper *)arg; http_free(rt); } void malloc_cleanup(void *arg) { - debug(1, "malloc cleanup called."); + // debug(1, "malloc cleanup called."); free(arg); } -- cgit v1.2.3