Improved libfast session management, using a hashtable

2 files changed, 109 insertions, 40 deletions

diff --git a/src/libfast/dispatcher.c b/src/libfast/dispatcher.c
index e99923d10..04fa33961 100644
--- a/src/libfast/dispatcher.c
+++ b/src/libfast/dispatcher.c
@@ -26,6 +26,10 @@
 #include <debug.h>
 #include <utils/mutex.h>
 #include <utils/linked_list.h>
+#include <utils/hashtable.h>
+
+/** Intervall to check for expired sessions, in seconds */
+#define CLEANUP_INTERVAL 30
 
 typedef struct private_dispatcher_t private_dispatcher_t;
 
@@ -60,9 +64,9 @@ struct private_dispatcher_t {
 	mutex_t *mutex;
 
 	/**
-	 * List of sessions
+	 * Hahstable with active sessions
 	 */
-	linked_list_t *sessions;
+	hashtable_t *sessions;
 
 	/**
 	 * session timeout
@@ -70,6 +74,11 @@ struct private_dispatcher_t {
 	time_t timeout;
 
 	/**
+	 * timestamp of last session cleanup round
+	 */
+	time_t last_cleanup;
+
+	/**
 	 * running in debug mode?
 	 */
 	bool debug;
@@ -219,6 +228,60 @@ static void add_filter(private_dispatcher_t *this,
 }
 
 /**
+ * Hashtable hash function
+ */
+static u_int session_hash(char *sid)
+{
+	return chunk_hash(chunk_create(sid, strlen(sid)));
+}
+
+/**
+ * Hashtable equals function
+ */
+static bool session_equals(char *sid1, char *sid2)
+{
+	return streq(sid1, sid2);
+}
+
+/**
+ * Cleanup unused sessions
+ */
+static void cleanup_sessions(private_dispatcher_t *this, time_t now)
+{
+	if (this->last_cleanup < now - CLEANUP_INTERVAL)
+	{
+		char *sid;
+		session_entry_t *entry;
+		enumerator_t *enumerator;
+		linked_list_t *remove;
+
+		this->last_cleanup = now;
+		remove = linked_list_create();
+		enumerator = this->sessions->create_enumerator(this->sessions);
+		while (enumerator->enumerate(enumerator, &sid, &entry))
+		{
+			/* check all sessions for timeout or close flag */
+			if (!entry->in_use &&
+				(entry->used < now - this->timeout || entry->closed))
+			{
+				remove->insert_last(remove, sid);
+			}
+		}
+		enumerator->destroy(enumerator);
+
+		while (remove->remove_last(remove, (void**)&sid) == SUCCESS)
+		{
+			entry = this->sessions->remove(this->sessions, sid);
+			if (entry)
+			{
+				session_entry_destroy(entry);
+			}
+		}
+		remove->destroy(remove);
+	}
+}
+
+/**
  * Actual dispatching code
  */
 static void dispatch(private_dispatcher_t *this)
@@ -228,8 +291,7 @@ static void dispatch(private_dispatcher_t *this)
 	while (TRUE)
 	{
 		request_t *request;
-		session_entry_t *current, *found = NULL;
-		enumerator_t *enumerator;
+		session_entry_t *found = NULL;
 		time_t now;
 		char *sid;
 
@@ -241,33 +303,18 @@ static void dispatch(private_dispatcher_t *this)
 		{
 			continue;
 		}
-		sid = request->get_cookie(request, "SID");
 		now = time_monotonic(NULL);
+		sid = request->get_cookie(request, "SID");
 
-		/* find session */
 		this->mutex->lock(this->mutex);
-		enumerator = this->sessions->create_enumerator(this->sessions);
-		while (enumerator->enumerate(enumerator, &current))
+		if (sid)
 		{
-			/* check all sessions for timeout or close flag
-			 * TODO: use a seperate cleanup thread */
-			if (!current->in_use &&
-				(current->used < now - this->timeout || current->closed))
-			{
-				this->sessions->remove_at(this->sessions, enumerator);
-				session_entry_destroy(current);
-				continue;
-			}
-			/* find by session ID. Prevent session hijacking by host check */
-			if (!found && sid && current->session->get_sid(current->session) &&
-				streq(current->session->get_sid(current->session), sid) &&
-				streq(current->host, request->get_host(request)))
-			{
-				found = current;
-			}
+			found = this->sessions->get(this->sessions, sid);
+		}
+		if (found && !streq(found->host, request->get_host(request)))
+		{
+			found = NULL;
 		}
-		enumerator->destroy(enumerator);
-
 		if (found)
 		{
 			/* wait until session is unused */
@@ -279,7 +326,8 @@ static void dispatch(private_dispatcher_t *this)
 		else
 		{	/* create a new session if not found */
 			found = session_entry_create(this, request->get_host(request));
-			this->sessions->insert_first(this->sessions, found);
+			sid = found->session->get_sid(found->session);
+			this->sessions->put(this->sessions, sid, found);
 		}
 		found->in_use = TRUE;
 		this->mutex->unlock(this->mutex);
@@ -292,10 +340,10 @@ static void dispatch(private_dispatcher_t *this)
 		this->mutex->lock(this->mutex);
 		found->in_use = FALSE;
 		found->closed = request->session_closed(request);
-		this->mutex->unlock(this->mutex);
 		found->cond->signal(found->cond);
+		cleanup_sessions(this, now);
+		this->mutex->unlock(this->mutex);
 
-		/* cleanup */
 		request->destroy(request);
 	}
 }
@@ -338,13 +386,23 @@ static void waitsignal(private_dispatcher_t *this)
  */
 static void destroy(private_dispatcher_t *this)
 {
+	char *sid;
+	session_entry_t *entry;
+	enumerator_t *enumerator;
+
 	FCGX_ShutdownPending();
 	while (this->thread_count--)
 	{
 		pthread_cancel(this->threads[this->thread_count]);
 		pthread_join(this->threads[this->thread_count], NULL);
 	}
-	this->sessions->destroy_function(this->sessions, (void*)session_entry_destroy);
+	enumerator = this->sessions->create_enumerator(this->sessions);
+	while (enumerator->enumerate(enumerator, &sid, &entry))
+	{
+		session_entry_destroy(entry);
+	}
+	enumerator->destroy(enumerator);
+	this->sessions->destroy(this->sessions);
 	this->controllers->destroy_function(this->controllers, free);
 	this->filters->destroy_function(this->filters, free);
 	this->mutex->destroy(this->mutex);
@@ -366,7 +424,8 @@ dispatcher_t *dispatcher_create(char *socket, bool debug, int timeout,
 	this->public.waitsignal = (void(*)(dispatcher_t*))waitsignal;
 	this->public.destroy = (void(*)(dispatcher_t*))destroy;
 
-	this->sessions = linked_list_create();
+	this->sessions = hashtable_create((void*)session_hash,
+									  (void*)session_equals, 4096);
 	this->controllers = linked_list_create();
 	this->filters = linked_list_create();
 	this->context_constructor = constructor;
@@ -374,6 +433,7 @@ dispatcher_t *dispatcher_create(char *socket, bool debug, int timeout,
 	this->param = param;
 	this->fd = 0;
 	this->timeout = timeout;
+	this->last_cleanup = time_monotonic(NULL);
 	this->debug = debug;
 	this->threads = NULL;
 
diff --git a/src/libfast/session.c b/src/libfast/session.c
index 06fcc3bba..f03b75542 100644
--- a/src/libfast/session.c
+++ b/src/libfast/session.c
@@ -23,6 +23,8 @@
 
 #include <utils/linked_list.h>
 
+#define COOKIE_LEN 16
+
 typedef struct private_session_t private_session_t;
 
 /**
@@ -38,7 +40,12 @@ struct private_session_t {
 	/**
 	 * session ID
 	 */
-	char *sid;
+	char sid[COOKIE_LEN * 2 + 1];
+
+	/**
+	 * have we sent the session cookie?
+	 */
+	bool cookie_sent;
 
 	/**
 	 * list of controller instances controller_t
@@ -75,19 +82,20 @@ static void add_filter(private_session_t *this, filter_t *filter)
 /**
  * Create a session ID and a cookie
  */
-static void create_sid(private_session_t *this, request_t *request)
+static void create_sid(private_session_t *this)
 {
-	char buf[16];
+	char buf[COOKIE_LEN];
 	rng_t *rng;
 
+	memset(buf, 0, sizeof(buf));
+	memset(this->sid, 0, sizeof(this->sid));
 	rng = lib->crypto->create_rng(lib->crypto, RNG_WEAK);
 	if (rng)
 	{
 		rng->get_bytes(rng, sizeof(buf), buf);
-		this->sid = chunk_to_hex(chunk_create(buf, sizeof(buf)), NULL, FALSE).ptr;
-		request->add_cookie(request, "SID", this->sid);
 		rng->destroy(rng);
 	}
+	chunk_to_hex(chunk_create(buf, sizeof(buf)), this->sid, FALSE);
 }
 
 /**
@@ -123,9 +131,10 @@ static void process(private_session_t *this, request_t *request)
 	controller_t *current;
 	int i = 0;
 
-	if (this->sid == NULL)
+	if (!this->cookie_sent)
 	{
-		create_sid(this, request);
+		request->add_cookie(request, "SID", this->sid);
+		this->cookie_sent = TRUE;
 	}
 
 	start = request->get_path(request);
@@ -189,7 +198,6 @@ static void destroy(private_session_t *this)
 	this->controllers->destroy_offset(this->controllers, offsetof(controller_t, destroy));
 	this->filters->destroy_offset(this->filters, offsetof(filter_t, destroy));
 	DESTROY_IF(this->context);
-	free(this->sid);
 	free(this);
 }
 
@@ -206,7 +214,8 @@ session_t *session_create(context_t *context)
 	this->public.get_sid = (char*(*)(session_t*))get_sid;
 	this->public.destroy = (void(*)(session_t*))destroy;
 
-	this->sid = NULL;
+	create_sid(this);
+	this->cookie_sent = FALSE;
 	this->controllers = linked_list_create();
 	this->filters = linked_list_create();
 	this->context = context;