From: Christian Hopps <chopps@labn.net>
Date: Wed, 3 May 2023 01:43:35 +0000 (-0400)
Subject: mgmtd: fix cleanup of cleanup in FE adapter code
X-Git-Tag: base_9.0~29^2~21
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=bcc350971f94f20bac8a86eab2bbcc604c6646da;p=mirror%2Ffrr.git

mgmtd: fix cleanup of cleanup in FE adapter code

Signed-off-by: Christian Hopps <chopps@labn.net>
---

diff --git a/mgmtd/mgmt_fe_adapter.c b/mgmtd/mgmt_fe_adapter.c
index b55bee0e97..f19f860c6d 100644
--- a/mgmtd/mgmt_fe_adapter.c
+++ b/mgmtd/mgmt_fe_adapter.c
@@ -240,6 +240,7 @@ static void mgmt_fe_cleanup_session(struct mgmt_fe_session_ctx **session)
 
 		mgmt_fe_sessions_del(&(*session)->adapter->fe_sessions,
 				     *session);
+		assert((*session)->adapter->refcount > 1);
 		mgmt_fe_adapter_unlock(&(*session)->adapter);
 	}
 
@@ -278,24 +279,6 @@ static bool mgmt_fe_session_hash_cmp(const void *d1, const void *d2)
 	return (session1->session_id == session2->session_id);
 }
 
-static void mgmt_fe_session_hash_free(void *data)
-{
-	struct mgmt_fe_session_ctx *session = data;
-
-	mgmt_fe_cleanup_session(&session);
-}
-
-static void mgmt_fe_session_hash_destroy(void)
-{
-	if (mgmt_fe_sessions == NULL)
-		return;
-
-	hash_clean(mgmt_fe_sessions,
-		   mgmt_fe_session_hash_free);
-	hash_free(mgmt_fe_sessions);
-	mgmt_fe_sessions = NULL;
-}
-
 static inline struct mgmt_fe_session_ctx *
 mgmt_session_id2ctx(uint64_t session_id)
 {
@@ -338,15 +321,6 @@ mgmt_fe_create_session(struct mgmt_fe_client_adapter *adapter,
 	return session;
 }
 
-static void
-mgmt_fe_cleanup_sessions(struct mgmt_fe_client_adapter *adapter)
-{
-	struct mgmt_fe_session_ctx *session;
-
-	FOREACH_SESSION_IN_LIST (adapter, session)
-		mgmt_fe_cleanup_session(&session);
-}
-
 static int mgmt_fe_adapter_send_msg(struct mgmt_fe_client_adapter *adapter,
 				    Mgmtd__FeMessage *fe_msg)
 {
@@ -645,24 +619,36 @@ mgmt_fe_find_adapter_by_name(const char *name)
 	return NULL;
 }
 
-
-static int mgmt_fe_adapter_notify_disconnect(struct msg_conn *conn)
+static void mgmt_fe_adapter_delete(struct mgmt_fe_client_adapter *adapter)
 {
-	struct mgmt_fe_client_adapter *adapter = conn->user;
+	struct mgmt_fe_session_ctx *session;
+	MGMTD_FE_ADAPTER_DBG("deleting client adapter '%s'", adapter->name);
 
 	/* TODO: notify about client disconnect for appropriate cleanup */
-	mgmt_fe_cleanup_sessions(adapter);
+	FOREACH_SESSION_IN_LIST (adapter, session)
+		mgmt_fe_cleanup_session(&session);
 	mgmt_fe_sessions_fini(&adapter->fe_sessions);
 
-	/* remove from list */
-	mgmt_fe_adapters_del(&mgmt_fe_adapters, adapter);
-
-	/* XXX do we expect this to free? if so then just free it :( */
+	assert(adapter->refcount == 1);
 	mgmt_fe_adapter_unlock(&adapter);
+}
+
+static int mgmt_fe_adapter_notify_disconnect(struct msg_conn *conn)
+{
+	struct mgmt_fe_client_adapter *adapter = conn->user;
+
+	MGMTD_FE_ADAPTER_DBG("notify disconnect for client adapter '%s'",
+			     adapter->name);
+
+	mgmt_fe_adapter_delete(adapter);
 
 	return 0;
 }
 
+/*
+ * XXX chopps: get rid of this, we should have deleted sessions when there was a
+ * disconnect
+ */
 static void
 mgmt_fe_adapter_cleanup_old_conn(struct mgmt_fe_client_adapter *adapter)
 {
@@ -683,17 +669,6 @@ mgmt_fe_adapter_cleanup_old_conn(struct mgmt_fe_client_adapter *adapter)
 	}
 }
 
-static void
-mgmt_fe_cleanup_adapters(void)
-{
-	struct mgmt_fe_client_adapter *adapter;
-
-	FOREACH_ADAPTER_IN_LIST (adapter) {
-		mgmt_fe_cleanup_sessions(adapter);
-		mgmt_fe_adapter_unlock(&adapter);
-	}
-}
-
 static int
 mgmt_fe_session_handle_lockds_req_msg(struct mgmt_fe_session_ctx *session,
 					  Mgmtd__FeLockDsReq *lockds_req)
@@ -1423,14 +1398,32 @@ void mgmt_fe_adapter_init(struct event_loop *tm)
 	}
 }
 
+static void mgmt_fe_abort_if_session(void *data)
+{
+	struct mgmt_fe_session_ctx *session = data;
+
+	MGMTD_FE_ADAPTER_ERR("found orphaned session id %" PRIu64
+			     " client id %" PRIu64 " adapter %s",
+			     session->session_id, session->client_id,
+			     session->adapter ? session->adapter->name
+					      : "NULL");
+	abort();
+}
+
 /*
  * Destroy the FE adapter module
  */
 void mgmt_fe_adapter_destroy(void)
 {
+	struct mgmt_fe_client_adapter *adapter;
+
 	msg_server_cleanup(&mgmt_fe_server);
-	mgmt_fe_cleanup_adapters();
-	mgmt_fe_session_hash_destroy();
+
+	/* Deleting the adapters will delete all the sessions */
+	FOREACH_ADAPTER_IN_LIST (adapter)
+		mgmt_fe_adapter_delete(adapter);
+
+	hash_clean_and_free(&mgmt_fe_sessions, mgmt_fe_abort_if_session);
 }
 
 /*