From 765a489db49cd9a512d53564ae22e0b2da9f9dd0 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 7 Dec 2021 19:37:01 +0000 Subject: [PATCH 1/6] Possibly fix a few memory leaks. Add a bogus exit to allow SPS to quit at the end of a play session, to make it easier to use valgrind. --- rtsp.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/rtsp.c b/rtsp.c index 7f78b211..076ba702 100644 --- a/rtsp.c +++ b/rtsp.c @@ -2298,7 +2298,7 @@ void handle_command(__attribute__((unused)) rtsp_conn_info *conn, rtsp_message * uint64_t length = 0; plist_get_data_val(the_item, &buff, &length); // debug(1,"Item %d, length: %" PRId64 " bytes", item_number, length); - if ((length >= strlen("bplist00")) && (strstr(buff, "bplist00") == buff)) { + if ((buff != NULL) && (length >= strlen("bplist00")) && (strstr(buff, "bplist00") == buff)) { // debug(1,"Contains a plist."); plist_t subsidiary_plist = NULL; plist_from_memory(buff, length, &subsidiary_plist); @@ -2315,6 +2315,8 @@ void handle_command(__attribute__((unused)) rtsp_conn_info *conn, rtsp_message * debug(1, "Can't access the plist!"); } } + if (buff != NULL) + free(buff); } } } else { @@ -2521,6 +2523,8 @@ void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_messag debug(1, "Connection %d: missing plist!", conn->connection_number); resp->respcode = 451; // don't know what to do here } + debug(1,"Bogus exit for valgrind."); + exit(EXIT_SUCCESS); // } #endif @@ -3080,6 +3084,7 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) plist_free(setupResponsePlist); msg_add_header(resp, "Content-Type", "application/x-apple-binary-plist"); } + plist_free(messagePlist); debug_log_rtsp_message(2, " SETUP response", resp); } #endif From 7258a45bea6cac73fe581f41df177470120b4dfa Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 7 Dec 2021 19:58:00 +0000 Subject: [PATCH 2/6] Fix a few more memory leaks --- mdns_avahi.c | 2 +- rtsp.c | 8 +++++++- shairport.c | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/mdns_avahi.c b/mdns_avahi.c index 95984419..7103b1ee 100644 --- a/mdns_avahi.c +++ b/mdns_avahi.c @@ -112,7 +112,7 @@ static void resolve_callback(AvahiServiceResolver *r, AVAHI_GCC_UNUSED AvahiIfIn char portstring[20]; memset(portstring, 0, sizeof(portstring)); snprintf(portstring, sizeof(portstring), "%u", port); - send_ssnc_metadata('dapo', strdup(portstring), strlen(portstring), 0); + send_ssnc_metadata('dapo', portstring, strlen(portstring), 0); #endif } } else { diff --git a/rtsp.c b/rtsp.c index 076ba702..6f46c3f5 100644 --- a/rtsp.c +++ b/rtsp.c @@ -2282,7 +2282,7 @@ void handle_command(__attribute__((unused)) rtsp_conn_info *conn, rtsp_message * if (item != NULL) { char *typeValue = NULL; plist_get_string_val(item, &typeValue); - if (strcmp(typeValue, "updateMRSupportedCommands") == 0) { + if ((typeValue != NULL) && (strcmp(typeValue, "updateMRSupportedCommands") == 0)) { item = plist_dict_get_item(command_dict, "params"); if (item != NULL) { // the item should be a dict @@ -2331,6 +2331,8 @@ void handle_command(__attribute__((unused)) rtsp_conn_info *conn, rtsp_message * "POST /command plist type is \"%s\", but \"updateMRSupportedCommands\" expected.", typeValue); } + if (typeValue != NULL) + free(typeValue); } else { debug(1, "Could not get the \"type\" item."); } @@ -2523,6 +2525,10 @@ void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_messag debug(1, "Connection %d: missing plist!", conn->connection_number); resp->respcode = 451; // don't know what to do here } + if (conn->dacp_active_remote != NULL) { + free(conn->dacp_active_remote); + conn->dacp_active_remote = NULL; + } debug(1,"Bogus exit for valgrind."); exit(EXIT_SUCCESS); // } diff --git a/shairport.c b/shairport.c index ecba335a..e2fd0b5e 100644 --- a/shairport.c +++ b/shairport.c @@ -1843,7 +1843,7 @@ int main(int argc, char **argv) { char *uuid = malloc(UUID_STR_LEN); // Produces a UUID string at uuid consisting of lower-case letters uuid_unparse_lower(binuuid, uuid); - config.airplay_pi = strdup(uuid); + config.airplay_pi = uuid; debug(1, "Started in Airplay 2 mode on device \"%s\"!", config.airplay_device_id); #else debug(1, "Started in Airplay 1 mode!"); From 9eea4e4c0c7b3ef0c740062d781bb8dc4f61693c Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 7 Dec 2021 20:05:37 +0000 Subject: [PATCH 3/6] Another memory leak. --- rtsp.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rtsp.c b/rtsp.c index 6f46c3f5..af7066f9 100644 --- a/rtsp.c +++ b/rtsp.c @@ -2529,6 +2529,10 @@ void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_messag free(conn->dacp_active_remote); conn->dacp_active_remote = NULL; } + if (conn->ap2_timing_peer_list_message) { + free(conn->ap2_timing_peer_list_message); + conn->ap2_timing_peer_list_message = NULL; + } debug(1,"Bogus exit for valgrind."); exit(EXIT_SUCCESS); // } From d9b6a28307cbea7656a1eee3b2a6c6b5ca7fce1d Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 7 Dec 2021 20:29:00 +0000 Subject: [PATCH 4/6] Add a bogus exit for AP1 to make it easier to evaluate valgrind results. --- rtsp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rtsp.c b/rtsp.c index af7066f9..4396a363 100644 --- a/rtsp.c +++ b/rtsp.c @@ -2533,7 +2533,7 @@ void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_messag free(conn->ap2_timing_peer_list_message); conn->ap2_timing_peer_list_message = NULL; } - debug(1,"Bogus exit for valgrind."); + debug(1,"Bogus exit for valgrind -- remember to comment it out!."); exit(EXIT_SUCCESS); // } #endif @@ -2559,6 +2559,8 @@ void handle_teardown(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message conn->connection_number); resp->respcode = 451; } + debug(1,"Bogus exit for valgrind -- remember to comment it out!."); + exit(EXIT_SUCCESS); } void handle_flush(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { From 31a34ae6846f464237f4cbb2293b1bbdd8b070bf Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 7 Dec 2021 20:34:23 +0000 Subject: [PATCH 5/6] Fix another memory leak. --- rtsp.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rtsp.c b/rtsp.c index 4396a363..2f7da6c5 100644 --- a/rtsp.c +++ b/rtsp.c @@ -2559,6 +2559,10 @@ void handle_teardown(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message conn->connection_number); resp->respcode = 451; } + if (conn->dacp_active_remote != NULL) { + free(conn->dacp_active_remote); + conn->dacp_active_remote = NULL; + } debug(1,"Bogus exit for valgrind -- remember to comment it out!."); exit(EXIT_SUCCESS); } From b938064baebf1f2e8193a03a95bc09e63cc9be78 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 7 Dec 2021 21:34:53 +0000 Subject: [PATCH 6/6] Add client and server ip metadata for AP2. Remove timing peer list and DACP code only for a full AP2 teardown. --- rtp.c | 5 ----- rtsp.c | 47 +++++++++++++++++++++++++++++------------------ 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/rtp.c b/rtp.c index 401f4dba..220bb919 100644 --- a/rtp.c +++ b/rtp.c @@ -1003,11 +1003,6 @@ void rtp_setup(SOCKADDR *local, SOCKADDR *remote, uint16_t cport, uint16_t tport conn->request_sent = 0; conn->rtp_running = 1; - -#ifdef CONFIG_METADATA - send_ssnc_metadata('clip', conn->client_ip_string, strlen(conn->client_ip_string), 1); - send_ssnc_metadata('svip', conn->self_ip_string, strlen(conn->self_ip_string), 1); -#endif } } diff --git a/rtsp.c b/rtsp.c index 2f7da6c5..e350b5a4 100644 --- a/rtsp.c +++ b/rtsp.c @@ -526,6 +526,7 @@ void release_hold_on_play_lock(__attribute__((unused)) rtsp_conn_info *conn) { } void release_play_lock(rtsp_conn_info *conn) { + debug(1, "Connection %d: release play lock.", conn->connection_number); debug_mutex_lock(&playing_conn_lock, 1000000, 3); if (playing_conn == conn) { // if we have the player playing_conn = NULL; // let it go @@ -535,6 +536,7 @@ void release_play_lock(rtsp_conn_info *conn) { } int get_play_lock(rtsp_conn_info *conn) { + debug(1, "Connection %d: request play lock.", conn->connection_number); // returns -1 if it failed, 0 if it succeeded and 1 if it succeeded but // interrupted an existing session int response = 0; @@ -590,7 +592,7 @@ int get_play_lock(rtsp_conn_info *conn) { } if ((have_the_player == 1) && (interrupting_current_session == 1)) { - debug(2, "Connection %d: Got player lock", conn->connection_number); + debug(1, "Connection %d: Got player lock", conn->connection_number); response = 1; } else { debug(1, "Connection %d: failed to get player lock", conn->connection_number); @@ -599,7 +601,7 @@ int get_play_lock(rtsp_conn_info *conn) { } if ((have_the_player) && (interrupting_current_session == 0)) { - debug(2, "Connection %d: got player lock.", conn->connection_number); + debug(1, "Connection %d: Got player lock.", conn->connection_number); response = 0; } return response; @@ -2486,6 +2488,14 @@ void teardown_phase_two(rtsp_conn_info *conn) { debug(2, "Connection %d: TEARDOWN mdns_update on %s.", conn->connection_number, get_category_string(conn->airplay_stream_category)); mdns_update(NULL, secondary_txt_records); + if (conn->dacp_active_remote != NULL) { + free(conn->dacp_active_remote); + conn->dacp_active_remote = NULL; + } + if (conn->ap2_timing_peer_list_message) { + free(conn->ap2_timing_peer_list_message); + conn->ap2_timing_peer_list_message = NULL; + } release_play_lock(conn); } } @@ -2509,10 +2519,10 @@ void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_messag // we are being asked to close a stream teardown_phase_one(conn); plist_free(streams); - debug(2, "Connection %d: Stream TEARDOWN complete", conn->connection_number); + debug(1, "Connection %d: TEARDOWN phase one complete", conn->connection_number); } else { teardown_phase_two(conn); - debug(2, "Connection %d: non-stream TEARDOWN complete", conn->connection_number); + debug(1, "Connection %d: TEARDOWN phase two complete", conn->connection_number); } //} else { // warn("Connection %d TEARDOWN received without having the player (no ANNOUNCE?)", @@ -2525,16 +2535,8 @@ void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_messag debug(1, "Connection %d: missing plist!", conn->connection_number); resp->respcode = 451; // don't know what to do here } - if (conn->dacp_active_remote != NULL) { - free(conn->dacp_active_remote); - conn->dacp_active_remote = NULL; - } - if (conn->ap2_timing_peer_list_message) { - free(conn->ap2_timing_peer_list_message); - conn->ap2_timing_peer_list_message = NULL; - } - debug(1,"Bogus exit for valgrind -- remember to comment it out!."); - exit(EXIT_SUCCESS); // + // debug(1,"Bogus exit for valgrind -- remember to comment it out!."); + // exit(EXIT_SUCCESS); // } #endif @@ -2563,8 +2565,8 @@ void handle_teardown(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message free(conn->dacp_active_remote); conn->dacp_active_remote = NULL; } - debug(1,"Bogus exit for valgrind -- remember to comment it out!."); - exit(EXIT_SUCCESS); + // debug(1,"Bogus exit for valgrind -- remember to comment it out!."); + // exit(EXIT_SUCCESS); } void handle_flush(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { @@ -2637,6 +2639,11 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) debug(2, "Connection %d: SETUP: PTP setup detected.", conn->connection_number); conn->airplay_stream_category = ptp_stream; conn->timing_type = ts_ptp; + get_play_lock(conn); +#ifdef CONFIG_METADATA + send_ssnc_metadata('clip', conn->client_ip_string, strlen(conn->client_ip_string), 1); + send_ssnc_metadata('svip', conn->self_ip_string, strlen(conn->self_ip_string), 1); +#endif } else if (strcmp(timingProtocolString, "NTP") == 0) { debug(1, "Connection %d: SETUP: NTP setup detected.", conn->connection_number); conn->airplay_stream_category = ntp_stream; @@ -2891,7 +2898,6 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) debug(2, "Connection %d: SETUP on %s. A \"streams\" array has been found", conn->connection_number, get_category_string(conn->airplay_stream_category)); if (conn->airplay_stream_category == ptp_stream) { - get_play_lock(conn); // get stream[0] plist_t stream0 = plist_array_get_item(streams, 0); @@ -3214,7 +3220,12 @@ void handle_setup(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { } else { debug(1, "Connection %d: SETUP doesn't contain a Transport header.", conn->connection_number); } - if (resp->respcode != 200) { + if (resp->respcode == 200) { +#ifdef CONFIG_METADATA + send_ssnc_metadata('clip', conn->client_ip_string, strlen(conn->client_ip_string), 1); + send_ssnc_metadata('svip', conn->self_ip_string, strlen(conn->self_ip_string), 1); +#endif + } else { debug(1, "Connection %d: SETUP error -- releasing the player lock.", conn->connection_number); release_play_lock(conn); }