From: Mike Brady Date: Tue, 17 Jul 2018 09:28:06 +0000 (+0100) Subject: Begin to try to get Shairport Sync to quit cleanly, relinquishing memory and ports... X-Git-Tag: 3.3RC0~286^2~18 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a4eaace76b07027f5832071de4ed44c726b91786;p=thirdparty%2Fshairport-sync.git Begin to try to get Shairport Sync to quit cleanly, relinquishing memory and ports etc. properly so that valgrind can be more useful. --- diff --git a/dbus-service.c b/dbus-service.c index 14ace35b..b16b22da 100644 --- a/dbus-service.c +++ b/dbus-service.c @@ -544,6 +544,14 @@ gboolean notify_loop_status_callback(ShairportSyncAdvancedRemoteControl *skeleto return TRUE; } +static gboolean on_handle_quit(ShairportSync *skeleton, GDBusMethodInvocation *invocation, + __attribute__((unused)) const gchar *command, + __attribute__((unused)) gpointer user_data) { + debug(1, "quit requested (native interface)"); + shairport_sync_complete_quit(skeleton, invocation); + return TRUE; +} + static gboolean on_handle_remote_command(ShairportSync *skeleton, GDBusMethodInvocation *invocation, const gchar *command, __attribute__((unused)) gpointer user_data) { @@ -589,6 +597,9 @@ static void on_dbus_name_acquired(GDBusConnection *connection, const gchar *name g_signal_connect(shairportSyncSkeleton, "notify::loudness-threshold", G_CALLBACK(notify_loudness_threshold_callback), NULL); + g_signal_connect(shairportSyncSkeleton, "handle-quit", + G_CALLBACK(on_handle_quit), NULL); + g_signal_connect(shairportSyncSkeleton, "handle-remote-command", G_CALLBACK(on_handle_remote_command), NULL); diff --git a/mpris-service.c b/mpris-service.c index 4901a819..982623e1 100644 --- a/mpris-service.c +++ b/mpris-service.c @@ -183,6 +183,13 @@ void mpris_metadata_watcher(struct metadata_bundle *argc, __attribute__((unused) // media_player2_player_set_volume(mprisPlayerPlayerSkeleton, metadata_store.speaker_volume); } +static gboolean on_handle_quit(MediaPlayer2 *skeleton, GDBusMethodInvocation *invocation, + __attribute__((unused)) gpointer user_data) { + debug(1,"quit requested (MPRIS interface)."); + media_player2_complete_quit(skeleton, invocation); + return TRUE; +} + static gboolean on_handle_next(MediaPlayer2Player *skeleton, GDBusMethodInvocation *invocation, __attribute__((unused)) gpointer user_data) { send_simple_dacp_command("nextitem"); @@ -243,7 +250,7 @@ static void on_mpris_name_acquired(GDBusConnection *connection, const gchar *nam media_player2_set_desktop_entry(mprisPlayerSkeleton, "shairport-sync"); media_player2_set_identity(mprisPlayerSkeleton, "Shairport Sync"); - media_player2_set_can_quit(mprisPlayerSkeleton, FALSE); + media_player2_set_can_quit(mprisPlayerSkeleton, TRUE); media_player2_set_can_raise(mprisPlayerSkeleton, FALSE); media_player2_set_has_track_list(mprisPlayerSkeleton, FALSE); media_player2_set_supported_uri_schemes(mprisPlayerSkeleton, empty_string_array); @@ -261,6 +268,8 @@ static void on_mpris_name_acquired(GDBusConnection *connection, const gchar *nam media_player2_player_set_can_seek(mprisPlayerPlayerSkeleton, FALSE); media_player2_player_set_can_control(mprisPlayerPlayerSkeleton, TRUE); + g_signal_connect(mprisPlayerSkeleton, "handle-quit", G_CALLBACK(on_handle_quit), NULL); + g_signal_connect(mprisPlayerPlayerSkeleton, "handle-play", G_CALLBACK(on_handle_play), NULL); g_signal_connect(mprisPlayerPlayerSkeleton, "handle-pause", G_CALLBACK(on_handle_pause), NULL); g_signal_connect(mprisPlayerPlayerSkeleton, "handle-play-pause", G_CALLBACK(on_handle_play_pause), diff --git a/org.gnome.ShairportSync.xml b/org.gnome.ShairportSync.xml index aa693fd5..1b734bff 100644 --- a/org.gnome.ShairportSync.xml +++ b/org.gnome.ShairportSync.xml @@ -1,6 +1,7 @@ + diff --git a/player.h b/player.h index 793476e6..295b0ccd 100644 --- a/player.h +++ b/player.h @@ -74,9 +74,10 @@ typedef struct { // otherwise int64_t maximum_latency; // set if an a=max-latency: line appears in the ANNOUNCE message; zero // otherwise - + int fd; int authorized; // set if a password is required and has been supplied + char *auth_nonce; // the session nonce, if needed stream_cfg stream; SOCKADDR remote, local; int stop; diff --git a/rtsp.c b/rtsp.c index e96c29ca..5b5a3428 100644 --- a/rtsp.c +++ b/rtsp.c @@ -1723,7 +1723,7 @@ static void apple_challenge(int fd, rtsp_message *req, rtsp_message *resp) { if (padding) *padding = 0; - msg_add_header(resp, "Apple-Response", encoded); + msg_add_header(resp, "Apple-Response", encoded); // will be freed when the response is freed. free(challresp); free(encoded); } @@ -1737,7 +1737,7 @@ static char *make_nonce(void) { if (read(fd, random, sizeof(random)) != sizeof(random)) debug(1, "Error reading /dev/random"); close(fd); - return base64_enc(random, 8); + return base64_enc(random, 8); // returns a pointer to malloc'ed memory } static int rtsp_auth(char **nonce, rtsp_message *req, rtsp_message *resp) { @@ -1888,6 +1888,48 @@ authenticate: return 1; } +void rtsp_conversation_thread_cleanup_function(void *arg) { + debug(1,"rtsp_conversation_thread_func_cleanup_function called."); + rtsp_conn_info *conn = (rtsp_conn_info *)arg; + if (conn->fd > 0) + close(conn->fd); + if (conn->auth_nonce) { + free(conn->auth_nonce); + conn->auth_nonce = NULL; + } + rtp_terminate(conn); + if (playing_conn == conn) { + debug(3, "Unlocking play lock on RTSP conversation thread %d.", conn->connection_number); + playing_conn = NULL; + pthread_mutex_unlock(&play_lock); + } + + if (conn->dacp_id) { + free(conn->dacp_id); + conn->dacp_id = NULL; + } + + debug(2, "Connection %d: RTSP thread terminated.", conn->connection_number); + conn->running = 0; + + // remove flow control and mutexes + int rc = pthread_cond_destroy(&conn->flowcontrol); + if (rc) + debug(1, "Connection %d: error %d destroying flow control condition variable.", + conn->connection_number, rc); + rc = pthread_mutex_destroy(&conn->ab_mutex); + if (rc) + debug(1, "Connection %d: error %d destroying ab_mutex.", conn->connection_number, rc); + rc = pthread_mutex_destroy(&conn->flush_mutex); + if (rc) + debug(1, "Connection %d: error %d destroying flush_mutex.", conn->connection_number, rc); + } + +void msg_cleanup_function(void *arg) { + debug(1,"msg_cleanup_function called."); + msg_free((rtsp_message *)arg); +} + static void *rtsp_conversation_thread_func(void *pconn) { rtsp_conn_info *conn = pconn; @@ -1912,26 +1954,29 @@ static void *rtsp_conversation_thread_func(void *pconn) { conn->connection_number, rc); rtp_initialise(conn); - - rtsp_message *req, *resp; - char *hdr, *auth_nonce = NULL; + char *hdr = NULL; enum rtsp_read_request_response reply; int rtsp_read_request_attempt_count = 1; // 1 means exit immediately + rtsp_message *req, *resp; + pthread_cleanup_push(rtsp_conversation_thread_cleanup_function,(void *)conn); while (conn->stop == 0) { int debug_level = 3; // for printing the request and response - reply = rtsp_read_request(conn, &req); + reply = rtsp_read_request(conn,&req); if (reply == rtsp_read_request_response_ok) { + pthread_cleanup_push(msg_cleanup_function,(void*)req); + resp = msg_init(); + pthread_cleanup_push(msg_cleanup_function,(void*)resp); + resp->respcode = 400; + if (strcmp(req->method, "OPTIONS") != 0) // the options message is very common, so don't log it until level 3 debug_level = 2; debug(debug_level, "RTSP thread %d received an RTSP Packet of type \"%s\":", conn->connection_number, req->method), debug_print_msg_headers(debug_level, req); - resp = msg_init(); - resp->respcode = 400; apple_challenge(conn->fd, req, resp); hdr = msg_get_header(req, "CSeq"); @@ -1940,7 +1985,7 @@ static void *rtsp_conversation_thread_func(void *pconn) { // msg_add_header(resp, "Audio-Jack-Status", "connected; type=analog"); msg_add_header(resp, "Server", "AirTunes/105.1"); - if ((conn->authorized == 1) || (rtsp_auth(&auth_nonce, req, resp)) == 0) { + if ((conn->authorized == 1) || (rtsp_auth(&conn->auth_nonce, req, resp)) == 0) { conn->authorized = 1; // it must have been authorized or didn't need a password struct method_handler *mh; int method_selected = 0; @@ -1967,8 +2012,8 @@ static void *rtsp_conversation_thread_func(void *pconn) { if (conn->stop == 0) { msg_write_response(conn->fd, resp); } - msg_free(req); - msg_free(resp); + pthread_cleanup_pop(1); + pthread_cleanup_pop(1); } else { int tstop = 0; if (reply == rtsp_read_request_response_immediate_shutdown_requested) @@ -2009,37 +2054,7 @@ static void *rtsp_conversation_thread_func(void *pconn) { } } } - - if (conn->fd > 0) - close(conn->fd); - if (auth_nonce) - free(auth_nonce); - rtp_terminate(conn); - if (playing_conn == conn) { - debug(3, "Unlocking play lock on RTSP conversation thread %d.", conn->connection_number); - playing_conn = NULL; - pthread_mutex_unlock(&play_lock); - } - - if (conn->dacp_id) { - free(conn->dacp_id); - conn->dacp_id = NULL; - } - - debug(2, "Connection %d: RTSP thread terminated.", conn->connection_number); - conn->running = 0; - - // remove flow control and mutexes - rc = pthread_cond_destroy(&conn->flowcontrol); - if (rc) - debug(1, "Connection %d: error %d destroying flow control condition variable.", - conn->connection_number, rc); - rc = pthread_mutex_destroy(&conn->ab_mutex); - if (rc) - debug(1, "Connection %d: error %d destroying ab_mutex.", conn->connection_number, rc); - rc = pthread_mutex_destroy(&conn->flush_mutex); - if (rc) - debug(1, "Connection %d: error %d destroying flush_mutex.", conn->connection_number, rc); + pthread_cleanup_pop(1); pthread_exit(NULL); }