From 271933a4d7438f73f1487842bb18b2442ceaec48 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Mon, 3 Oct 2011 15:23:16 -0400 Subject: [PATCH] Fix session list deadlock On session destroy, the session list lock is acquired but this call could be made inside a section where this lock is already acquired. The call has been changed to using the session pointer and not the name thus removing a useless list walk. Signed-off-by: David Goulet --- ltt-sessiond/main.c | 2 +- ltt-sessiond/session.c | 27 ++++++++++++--------------- ltt-sessiond/session.h | 2 +- tests/test_sessions.c | 19 ++++++++----------- 4 files changed, 22 insertions(+), 28 deletions(-) diff --git a/ltt-sessiond/main.c b/ltt-sessiond/main.c index c66ecb956..edeb86d29 100644 --- a/ltt-sessiond/main.c +++ b/ltt-sessiond/main.c @@ -2312,7 +2312,7 @@ static int cmd_destroy_session(struct ltt_session *session, char *name) perror("write kernel poll pipe"); } - ret = session_destroy(name); + ret = session_destroy(session); return ret; } diff --git a/ltt-sessiond/session.c b/ltt-sessiond/session.c index b13f4a916..ae83e45ae 100644 --- a/ltt-sessiond/session.c +++ b/ltt-sessiond/session.c @@ -144,23 +144,20 @@ struct ltt_session *session_find_by_name(char *name) * * Return -1 if no session is found. On success, return 1; */ -int session_destroy(char *name) +int session_destroy(struct ltt_session *session) { - struct ltt_session *iter, *tmp; - - session_lock_list(); - cds_list_for_each_entry_safe(iter, tmp, <t_session_list.head, list) { - if (strcmp(iter->name, name) == 0) { - DBG("Destroying session %s", iter->name); - del_session_list(iter); - free(iter->name); - free(iter->path); - pthread_mutex_destroy(&iter->lock); - free(iter); - break; - } + /* Safety check */ + if (session == NULL) { + ERR("Session pointer was null on session destroy"); + return LTTCOMM_OK; } - session_unlock_list(); + + DBG("Destroying session %s", session->name); + del_session_list(session); + free(session->name); + free(session->path); + pthread_mutex_destroy(&session->lock); + free(session); return LTTCOMM_OK; } diff --git a/ltt-sessiond/session.h b/ltt-sessiond/session.h index bbbbffc45..2f40f585d 100644 --- a/ltt-sessiond/session.h +++ b/ltt-sessiond/session.h @@ -71,7 +71,7 @@ struct ltt_session { /* Prototypes */ int session_create(char *name, char *path); -int session_destroy(char *name); +int session_destroy(struct ltt_session *session); void session_lock(struct ltt_session *session); void session_lock_list(void); diff --git a/tests/test_sessions.c b/tests/test_sessions.c index 8ea365fed..3e9bbbbb5 100644 --- a/tests/test_sessions.c +++ b/tests/test_sessions.c @@ -142,15 +142,18 @@ static int create_one_session(char *name, char *path) /* * Test deletion of 1 session */ -static int destroy_one_session(char *name) +static int destroy_one_session(struct ltt_session *session) { int ret; - ret = session_destroy(name); + ret = session_destroy(session); if (ret == LTTCOMM_OK) { /* Validate */ - ret = find_session_name(name); + if (session == NULL) { + return 0; + } + ret = find_session_name(session->name); if (ret < 0) { /* Success, -1 means that the sesion is NOT found */ return 0; @@ -201,12 +204,6 @@ static int fuzzing_destroy_args(void) return -1; } - ret = destroy_one_session(OVERFLOW_SESSION_NAME); - if (ret > 0) { - printf("Session destroyed with %s\n", OVERFLOW_SESSION_NAME); - return -1; - } - /* Session list must be 0 */ assert(!session_list->count); @@ -275,7 +272,7 @@ int main(int argc, char **argv) PRINT_OK(); printf("Destroy 1 session %s: ", SESSION1); - ret = destroy_one_session(SESSION1); + ret = destroy_one_session(tmp); if (ret < 0) { return -1; } @@ -319,7 +316,7 @@ int main(int argc, char **argv) printf("Destroying %d sessions: ", MAX_SESSIONS); for (i = 0; i < MAX_SESSIONS; i++) { cds_list_for_each_entry_safe(iter, tmp, &session_list->head, list) { - ret = destroy_one_session(iter->name); + ret = destroy_one_session(iter); if (ret < 0) { printf("session %d (name: %s) creation failed\n", i, iter->name); return -1; -- 2.34.1