Thread safety and locking checks.

1 files changed, 57 insertions, 30 deletions

diff --git a/daemon/ldap.c b/daemon/ldap.c
index 956c464..f04879a 100644
--- a/daemon/ldap.c
+++ b/daemon/ldap.c
@@ -49,7 +49,7 @@ typedef struct ldap_context
     /* Base Handler ------------------------------------------------------ */
     bd_context_t bd;
 
-    /* Settings ---------------------------------------------------------- */
+    /* Read Only --------------------------------------------------------- */
     const char* servers;      /* Servers to authenticate against (required) */
     const char* filter;       /* Filter (either this or dnmap must be set) */
     const char* base;         /* Base for the filter */
@@ -65,7 +65,7 @@ typedef struct ldap_context
     int ldap_max;             /* Number of open connections allowed */
     int ldap_timeout;         /* Maximum amount of time to dedicate to an ldap query */
 
-    /* Context ----------------------------------------------------------- */
+    /* Require Locking --------------------------------------------------- */
     LDAP** pool;            /* Pool of available connections */
     int pool_mark;          /* Amount of connections allocated */
 }
@@ -421,27 +421,40 @@ static int validate_ldap_ha1(const ha_request_t* rq, ldap_context_t* ctx, LDAP*
 static LDAP* get_ldap_connection(const ha_request_t* rq, ldap_context_t* ctx)
 {
     LDAP* ld;
-    int i, r;
+    int i, r, create = 1;
 
     ASSERT(ctx);
 
-    for(i = 0; i < ctx->ldap_max; i++)
-    {
-        /* An open connection in the pool */
-        if(ctx->pool[i])
+    /*
+     * Note that below there maybe a race condition between the two locks
+     * but this will only allow a few extra connections to open at best
+     * and as such really isn't a big issue.
+     */
+
+    ha_lock(NULL);
+
+        for(i = 0; i < ctx->ldap_max; i++)
         {
-            ha_messagex(rq, LOG_DEBUG, "using cached LDAP connection");
-            ld = ctx->pool[i];
-            ctx->pool[i] = NULL;
-            return ld;
+            /* An open connection in the pool */
+            if(ctx->pool[i])
+            {
+                ha_messagex(rq, LOG_DEBUG, "using cached LDAP connection");
+                ld = ctx->pool[i];
+                ctx->pool[i] = NULL;
+                break;
+            }
         }
-    }
 
-    if(ctx->pool_mark >= ctx->ldap_max)
-    {
-        ha_messagex(rq, LOG_ERR, "too many open LDAP connections");
-        return NULL;
-    }
+        if(ld == NULL && ctx->pool_mark >= ctx->ldap_max)
+        {
+            ha_messagex(rq, LOG_ERR, "too many open LDAP connections");
+            create = 0;
+        }
+
+    ha_unlock(NULL);
+
+    if(ld != NULL || create == 0)
+        return ld;
 
     ld = ldap_init(ctx->servers, ctx->port);
     if(!ld)
@@ -462,16 +475,26 @@ static LDAP* get_ldap_connection(const ha_request_t* rq, ldap_context_t* ctx)
         }
     }
 
-    ctx->pool_mark++;
-    ha_messagex(rq, LOG_DEBUG, "opened new connection (total %d)", ctx->pool_mark);
+    ha_lock(NULL);
+
+        ctx->pool_mark++;
+        ha_messagex(rq, LOG_DEBUG, "opened new connection (total %d)", ctx->pool_mark);
+
+    ha_unlock(NULL);
+
     return ld;
 }
 
 static void discard_ldap_connection(const ha_request_t* rq, ldap_context_t* ctx, LDAP* ld)
 {
     ldap_unbind_s(ld);
-    ctx->pool_mark--;
-    ha_messagex(rq, LOG_DEBUG, "discarding connection (total %d)", ctx->pool_mark);
+
+    ha_lock(NULL);
+
+        ctx->pool_mark--;
+        ha_messagex(rq, LOG_DEBUG, "discarding connection (total %d)", ctx->pool_mark);
+
+    ha_unlock(NULL);
 }
 
 static void save_ldap_connection(const ha_request_t* rq, ldap_context_t* ctx, LDAP* ld)
@@ -494,17 +517,21 @@ static void save_ldap_connection(const ha_request_t* rq, ldap_context_t* ctx, LD
         break;
 
     default:
-        for(i = 0; i < ctx->ldap_max; i++)
-        {
-            /* An open connection in the pool */
-            if(!ctx->pool[i])
+        ha_lock(NULL);
+
+            for(i = 0; i < ctx->ldap_max; i++)
             {
-                ha_messagex(rq, LOG_DEBUG, "caching ldap connection for later use");
-                ctx->pool[i] = ld;
-                ld = NULL;
-                break;
+                /* An open connection in the pool */
+                if(!ctx->pool[i])
+                {
+                    ha_messagex(rq, LOG_DEBUG, "caching ldap connection for later use");
+                    ctx->pool[i] = ld;
+                    ld = NULL;
+                    break;
+                }
             }
-        }
+
+        ha_unlock(NULL);
         break;
     };