From: David Goulet <david.goulet@polymtl.ca>
Date: Mon, 3 Oct 2011 19:23:16 +0000 (-0400)
Subject: Fix session list deadlock
X-Git-Tag: v2.0-pre15~199
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=271933a4d7438f73f1487842bb18b2442ceaec48;p=lttng-tools.git

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 <david.goulet@polymtl.ca>
---

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, &ltt_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;