session: safe delete sessions

Signed-off-by: Nicolás A. Ortega Froysa <nicolas.ortega@zevenet.com>

include/session.h

@@ -19,9 +19,9 @@
 #define _ZPROXY_SESSION_H_
 
 #include <pthread.h>
+#include <stdbool.h>
 #include "list.h"
 #include "config.h"
-#include "http_request.h"
 
 #define MAX_SESSION_ID 255
 #define HASH_SESSION_SLOTS 512
@@ -42,14 +42,44 @@ struct zproxy_session_node {
 	// last_seen is used to calculate if the session has expired.
 	// If it has the value 0 means that the session does not expired, it is permanent
 	unsigned int timestamp;
+	bool defunct;
+	int refcnt;
 };
 
 
 struct zproxy_sessions *zproxy_sessions_alloc(const struct zproxy_service_cfg *service_cfg);
 void zproxy_sessions_flush(struct zproxy_sessions *sessions);
 void zproxy_sessions_free(struct zproxy_sessions *sessions);
-struct zproxy_session_node *zproxy_session_get(struct zproxy_sessions *sessions, const char *key);
-struct zproxy_session_node *zproxy_session_add(struct zproxy_sessions *sessions, const char *key, const struct sockaddr_in *bck);
+/**
+ * @brief Get a reference to session with provided key.
+ *
+ * @param sessions Service sessions structure to which the session belongs.
+ * @param key Unique key that identifies the session.
+ *
+ * @return A pointer to the session that must be released with
+ * zproxy_session_release(). If not found it will return NULL.
+ */
+struct zproxy_session_node *
+zproxy_session_get(struct zproxy_sessions *sessions, const char *key);
+/**
+ * @brief Create/Add a new session.
+ *
+ * @param sessions Service sessions structure to which the session will belong.
+ * @param key Unique key that identifies the session.
+ * @param bck Backend associated with the new session.
+ *
+ * @return A pointer to the session that must be released with
+ * zproxy_session_release(). If fails to add it will return NULL.
+ */
+struct zproxy_session_node *
+zproxy_session_add(struct zproxy_sessions *sessions, const char *key,
+		   const struct sockaddr_in *bck);
+/**
+ * @brief Release reference to the session.
+ *
+ * @param session Session to release.
+ */
+void zproxy_session_release(struct zproxy_session_node **session);
 void zproxy_sessions_remove_expired(struct zproxy_sessions *sessions);
 void zproxy_session_delete_backend(struct zproxy_sessions *sessions, const struct sockaddr_in *bck);
 int zproxy_session_delete(struct zproxy_sessions *sessions, const char *key);

include/state.h

@@ -20,7 +20,11 @@
 
 #include "list.h"
 #include "session.h"
+#include "zcu_log.h"
 #include <sys/time.h>
+#include <string>
+#include <unordered_map>
+#include <memory>
 #include <ev.h>
 #include <atomic>
 

src/ctl_handler.c

@@ -20,6 +20,7 @@
 #include "session.h"
 #include "state.h"
 #include "zcu_network.h"
+#include "zcu_log.h"
 #include "zproxy.h"
 #include "ctl.h"
 #include "zcu_http.h"
@@ -333,6 +334,7 @@ static enum ws_responses handle_patch(const std::string &req_path,
 				return WS_HTTP_500;
 			}
 			sess->timestamp = i.last_seen;
+			zproxy_session_release(&sess);
 		}
 
 		zproxy_state_release(&state);
@@ -459,6 +461,7 @@ static enum ws_responses handle_patch(const std::string &req_path,
 						return WS_HTTP_500;
 					}
 					sess->timestamp = k.last_seen;
+					zproxy_session_release(&sess);
 				}
 			}
 			zproxy_state_release(&state);
@@ -611,7 +614,8 @@ static enum ws_responses handle_put(const std::string &req_path,
 			return WS_HTTP_404;
 		}
 
-		zproxy_session_node *session = zproxy_session_add(sessions, sess_id, &backend->runtime.addr);
+		zproxy_session_node *session =
+			zproxy_session_add(sessions, sess_id, &backend->runtime.addr);
 		if (!session) {
 			zproxy_state_release(&state);
 			*resp_buf = zproxy_json_return_err("Unable to create session. Perhaps it already exists.");
@@ -619,6 +623,7 @@ static enum ws_responses handle_put(const std::string &req_path,
 		}
 		if (last_seen >= 0)
 			session->timestamp = last_seen;
+		zproxy_session_release(&session);
 		zproxy_state_release(&state);
 	} else if (zproxy_regex_exec(API_REGEX_SELECT_SERVICE_BACKENDS,
 				     req_path.c_str(), matches)) {

src/http_manager.cpp

@@ -575,6 +575,7 @@ validation::REQUEST_RESULT http_manager::validateRequest(HttpStream *stream)
 
 static void zproxy_http_manage_set_cookie(HttpStream *stream, std::string session_key)
 {
+	struct zproxy_session_node *session;
 	if (!stream->backend_config)
 		return;
 
@@ -588,9 +589,9 @@ static void zproxy_http_manage_set_cookie(HttpStream *stream, std::string sessio
 								stream->session->id);
 	}
 
-	zproxy_session_add(stream->session,
-						session_key.data(),
-						&stream->backend_config->runtime.addr);
+	session = zproxy_session_add(stream->session, session_key.data(),
+				     &stream->backend_config->runtime.addr);
+	zproxy_session_release(&session);
 }
 
 validation::REQUEST_RESULT http_manager::validateResponse(HttpStream *stream)

src/json.cpp

@@ -137,6 +137,8 @@ static json_t *serialize_service_sessions(const struct zproxy_service_cfg *servi
 	pthread_mutex_lock(&sessions->sessions_mutex);
 	for (int i = 0; i < HASH_SESSION_SLOTS; i++) {
 		list_for_each_entry(session, &sessions->session_hashtable[i], hlist) {
+			if (session->defunct)
+				continue;
 			json_array_append_new(jsessions,
 					      serialize_session(session, service_cfg));
 		}

src/service.c

@@ -498,6 +498,7 @@ zproxy_service_select_backend(struct zproxy_service_cfg *service_config,
 		session = zproxy_session_get(sessions, session_key.data());
 		if (session) {
 			selected_backend = zproxy_service_backend_session(service_config, &session->bck_addr, http_state);
+			zproxy_session_release(&session);
 			if (selected_backend)
 				return selected_backend;
 		}
@@ -506,8 +507,10 @@ zproxy_service_select_backend(struct zproxy_service_cfg *service_config,
 	selected_backend = (struct zproxy_backend_cfg *)zproxy_service_schedule(service_config, http_state);
 
 	if (selected_backend && !session_key.empty() &&
-		service_config->session.sess_type != SESS_TYPE::SESS_NONE)
-		zproxy_session_add(sessions, session_key.data(), &selected_backend->runtime.addr);
+		service_config->session.sess_type != SESS_TYPE::SESS_NONE) {
+		session = zproxy_session_add(sessions, session_key.data(), &selected_backend->runtime.addr);
+		zproxy_session_release(&session);
+	}
 
 	return selected_backend;
 }

src/session.c

@@ -18,6 +18,7 @@
 #include "session.h"
 #include "djb_hash.h"
 #include <time.h>
+#include <sys/syslog.h>
 
 static void zproxy_sessions_dump(struct zproxy_sessions *sessions)
 {
@@ -46,10 +47,16 @@ static int zproxy_session_is_expired(struct zproxy_session_node *session, unsign
 	return time(NULL) - session->timestamp > ttl;
 }
 
-static void zproxy_session_free(struct zproxy_session_node *session)
+static int zproxy_session_free(struct zproxy_session_node *session)
 {
-	list_del(&session->hlist);
-	free(session);
+	if (session->refcnt == 0) {
+		list_del(&session->hlist);
+		free(session);
+		return 1;
+	} else {
+		session->defunct = true;
+		return -1;
+	}
 }
 
 struct zproxy_sessions *zproxy_sessions_alloc(const struct zproxy_service_cfg *service_cfg)
@@ -79,9 +86,10 @@ void zproxy_sessions_flush(struct zproxy_sessions *sessions)
 
 	pthread_mutex_lock(&sessions->sessions_mutex);
 	sessions->size = 0;
-	for (i = 0; i < HASH_SESSION_SLOTS; i++)
+	for (i = 0; i < HASH_SESSION_SLOTS; i++) {
 		list_for_each_entry_safe(session, next, &sessions->session_hashtable[i], hlist)
 			zproxy_session_free(session);
+	}
 	pthread_mutex_unlock(&sessions->sessions_mutex);
 }
 
@@ -97,9 +105,8 @@ static struct zproxy_session_node *_zproxy_session_get(struct zproxy_sessions *s
 	int hash = djb_hash(key) % HASH_SESSION_SLOTS;
 
 	list_for_each_entry(cur, &sessions->session_hashtable[hash], hlist) {
-		if (strncmp(cur->key, key, strlen(cur->key)+1) == 0) {
+		if (!cur->defunct && strncmp(cur->key, key, strlen(cur->key)+1) == 0)
 			return cur;
-		}
 	}
 
 	return NULL;
@@ -111,6 +118,8 @@ struct zproxy_session_node *zproxy_session_get(struct zproxy_sessions *sessions,
 
 	pthread_mutex_lock(&sessions->sessions_mutex);
 	session = _zproxy_session_get(sessions, key);
+	if (session)
+		session->refcnt++;
 	pthread_mutex_unlock(&sessions->sessions_mutex);
 
 	return session;
@@ -121,34 +130,42 @@ struct zproxy_session_node *zproxy_session_add(struct zproxy_sessions *sessions,
 	struct zproxy_session_node *session;
 	uint32_t hash;
 
-	pthread_mutex_lock(&sessions->sessions_mutex);
-	session = _zproxy_session_get(sessions, key);
+	session = zproxy_session_get(sessions, key);
 	if (session) {
 		if (!zproxy_session_is_static(session))
 			zproxy_session_update_timestamp(session);
-		pthread_mutex_unlock(&sessions->sessions_mutex);
 		return session;
 	}
 
 	session = (struct zproxy_session_node *)calloc(1, sizeof(*session));
-	if (!session) {
-		pthread_mutex_unlock(&sessions->sessions_mutex);
+	if (!session)
 		return NULL;
-	}
 
 	snprintf(session->key, sizeof(session->key), "%s", key);
 	memcpy(&session->bck_addr, bck, sizeof(struct sockaddr_in));
 	session->timestamp = time(NULL);
+	session->defunct = false;
 
 	sessions->size++;
 
 	hash = djb_hash(session->key) % HASH_SESSION_SLOTS;
+	pthread_mutex_lock(&sessions->sessions_mutex);
 	list_add_tail(&session->hlist, &sessions->session_hashtable[hash]);
 	pthread_mutex_unlock(&sessions->sessions_mutex);
 
+	session->refcnt++;
+
 	return session;
 }
 
+void zproxy_session_release(struct zproxy_session_node **session)
+{
+	if (!*session)
+		return;
+
+	(*session)->refcnt--;
+}
+
 void zproxy_sessions_remove_expired(struct zproxy_sessions *sessions)
 {
 	struct zproxy_session_node *session, *next;
@@ -158,8 +175,8 @@ void zproxy_sessions_remove_expired(struct zproxy_sessions *sessions)
 	for (i = 0; i < HASH_SESSION_SLOTS; i++) {
 		list_for_each_entry_safe(session, next, &sessions->session_hashtable[i], hlist) {
 			if (zproxy_session_is_expired(session, sessions->ttl)) {
-				sessions->size--;
-				zproxy_session_free(session);
+				if (zproxy_session_free(session) < 0)
+					sessions->size--;
 			}
 		}
 	}
@@ -174,8 +191,8 @@ static int _zproxy_session_delete(struct zproxy_sessions *sessions, const char *
 	if (!session)
 		return -1;
 
-	sessions->size--;
-	zproxy_session_free(session);
+	if (zproxy_session_free(session) < 0)
+		sessions->size--;
 
 	return 0;
 }
@@ -218,8 +235,8 @@ void zproxy_session_delete_backend(struct zproxy_sessions *sessions, const struc
 	for (i = 0; i < HASH_SESSION_SLOTS; i++) {
 		list_for_each_entry_safe(session, next, &sessions->session_hashtable[i], hlist) {
 			if (memcmp(&session->bck_addr, bck, sizeof(struct sockaddr_in)) == 0) {
-				sessions->size--;
-				zproxy_session_free(session);
+				if (zproxy_session_free(session) < 0)
+					sessions->size--;
 			}
 		}
 	}
@@ -235,8 +252,8 @@ void zproxy_session_delete_old_backends(const struct zproxy_service_cfg *service
 	for (i = 0; i < HASH_SESSION_SLOTS; i++) {
 		list_for_each_entry_safe(session, next, &sessions->session_hashtable[i], hlist) {
 			if (!zproxy_backend_cfg_lookup(service, &session->bck_addr)) {
-				sessions->size--;
-				zproxy_session_free(session);
+				if (zproxy_session_free(session) < 0)
+					sessions->size--;
 			}
 		}
 	}