From 53e4b851883571c92073c87986759bd98dab9c7e Mon Sep 17 00:00:00 2001 From: Stef Walter Date: Wed, 18 Aug 2004 22:47:38 +0000 Subject: Thread safety and locking checks. --- daemon/bd.h | 10 ++-- daemon/ldap.c | 87 ++++++++++++++++++++++------------ daemon/mysql.c | 127 +++++++++++++++++++++++++++++++++++--------------- daemon/ntlm.c | 38 +++++++-------- daemon/pgsql.c | 89 +++++++++++++++++++++++------------ daemon/simple.c | 2 +- sample/httpauthd.conf | 4 +- 7 files changed, 232 insertions(+), 125 deletions(-) diff --git a/daemon/bd.h b/daemon/bd.h index a59b1e1..5199615 100644 --- a/daemon/bd.h +++ b/daemon/bd.h @@ -44,16 +44,18 @@ typedef void (*bd_escape_value)(const ha_request_t* rq, ha_buffer_t* buf, */ typedef struct bd_context { - hsh_t* cache; /* Some cached records or basic */ - + /* Readonly ---------------------------------------------*/ bd_validate_digest f_validate_digest; bd_validate_basic f_validate_basic; bd_escape_value f_escape_value; + + /* Require locking --------------------------------------*/ + hsh_t* cache; /* Some cached records or basic */ } bd_context_t; -#define BD_CALLBACKS(a, b, c) { NULL, (a), (b), (c) } -#define BD_DEFAULTS { NULL, NULL, NULL } +#define BD_CALLBACKS(a, b, c) {(a), (b), (c), NULL } +#define BD_DEFAULTS { NULL, NULL, NULL, NULL } /* ---------------------------------------------------------------------------------- * Base Handler Functions 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; }; diff --git a/daemon/mysql.c b/daemon/mysql.c index 0ed8f35..1a75374 100644 --- a/daemon/mysql.c +++ b/daemon/mysql.c @@ -26,7 +26,7 @@ typedef struct mysql_context /* Base Handler ------------------------------------------------------ */ bd_context_t bd; - /* Settings ---------------------------------------------------------- */ + /* Readonly Settings ------------------------------------------------- */ const char* host; /* The connection host or path */ unsigned int port; /* The connection port */ const char* user; /* The pgsql user name */ @@ -41,7 +41,7 @@ typedef struct mysql_context int mysql_timeout; /* Maximum amount of time to dedicate to a query */ int use_unix_socket; - /* Context ----------------------------------------------------------- */ + /* Require Locking --------------------------------------------------- */ MYSQL** pool; /* Pool of available connections */ int pool_mark; /* Amount of connections allocated */ } @@ -73,6 +73,11 @@ static const mysql_context_t mysql_defaults = 0 /* pool_mark */ }; +/* mysql_real_connect is not always thread-safe :( */ +static pthread_mutex_t g_mysql_mutex; +static pthread_mutexattr_t g_mysql_mutexattr; + + /* ------------------------------------------------------------------------------- * Internal Functions @@ -235,27 +240,40 @@ static MYSQL* get_mysql_connection(const ha_request_t* rq, mysql_context_t* ctx) { MYSQL* my; MYSQL* r; - int i; + int i, create = 1; ASSERT(ctx); - for(i = 0; i < ctx->mysql_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(&g_mysql_mutex); + + for(i = 0; i < ctx->mysql_max; i++) { - ha_messagex(rq, LOG_DEBUG, "using cached mysql connection"); - my = ctx->pool[i]; - ctx->pool[i] = NULL; - return my; + /* An open connection in the pool */ + if(ctx->pool[i]) + { + ha_messagex(rq, LOG_DEBUG, "using cached mysql connection"); + my = ctx->pool[i]; + ctx->pool[i] = NULL; + } } - } - if(ctx->pool_mark >= ctx->mysql_max) - { - ha_messagex(rq, LOG_ERR, "too many open mysql connections"); - return NULL; - } + + if(my == NULL && ctx->pool_mark >= ctx->mysql_max) + { + ha_messagex(rq, LOG_ERR, "too many open mysql connections"); + create = 0; + } + + ha_unlock(&g_mysql_mutex); + + if(my != NULL || create == 0) + return my; my = mysql_init(NULL); if(!my) @@ -269,29 +287,39 @@ static MYSQL* get_mysql_connection(const ha_request_t* rq, mysql_context_t* ctx) mysql_options(my, MYSQL_OPT_WRITE_TIMEOUT, (char*)&(ctx->mysql_timeout)); */ /* Apparently mysql_real_connect is not thread safe :( */ - ha_lock(NULL); + ha_lock(&g_mysql_mutex); + r = mysql_real_connect(my, ctx->use_unix_socket ? NULL : ctx->host, ctx->user, ctx->password, ctx->database, ctx->port, ctx->use_unix_socket ? ctx->host : NULL, 0); - ha_unlock(NULL); - if(!r) - { - ha_messagex(rq, LOG_ERR, "error opening mysql connection: %s", mysql_error(my)); - mysql_close(my); - return NULL; - } + if(!r) + { + ha_messagex(rq, LOG_ERR, "error opening mysql connection: %s", mysql_error(my)); + mysql_close(my); + my = NULL; + } + else + { + ctx->pool_mark++; + ha_messagex(rq, LOG_DEBUG, "opened new mysql connection (total %d)", ctx->pool_mark); + } + + ha_unlock(&g_mysql_mutex); - ctx->pool_mark++; - ha_messagex(rq, LOG_DEBUG, "opened new mysql connection (total %d)", ctx->pool_mark); return my; } static void discard_mysql_connection(const ha_request_t* rq, mysql_context_t* ctx, MYSQL* my) { mysql_close(my); - ctx->pool_mark--; - ha_messagex(rq, LOG_DEBUG, "discarding mysql connection (total %d)", ctx->pool_mark); + + ha_lock(&g_mysql_mutex); + + ctx->pool_mark--; + ha_messagex(rq, LOG_DEBUG, "discarding mysql connection (total %d)", ctx->pool_mark); + + ha_unlock(&g_mysql_mutex); } static void save_mysql_connection(const ha_request_t* rq, mysql_context_t* ctx, MYSQL* my) @@ -322,17 +350,21 @@ static void save_mysql_connection(const ha_request_t* rq, mysql_context_t* ctx, /* Make sure it's worth saving */ default: - for(i = 0; i < ctx->mysql_max; i++) - { - /* An open connection in the pool */ - if(!ctx->pool[i]) + ha_lock(&g_mysql_mutex); + + for(i = 0; i < ctx->mysql_max; i++) { - ha_messagex(rq, LOG_DEBUG, "caching mysql connection for later use"); - ctx->pool[i] = my; - my = NULL; - break; + /* An open connection in the pool */ + if(!ctx->pool[i]) + { + ha_messagex(rq, LOG_DEBUG, "caching mysql connection for later use"); + ctx->pool[i] = my; + my = NULL; + break; + } } - } + + ha_unlock(&g_mysql_mutex); break; }; @@ -701,6 +733,19 @@ int mysql_initialize(ha_context_t* context) ha_messagex(NULL, LOG_INFO, "initialized mysql handler"); } + /* Global Initialization */ + else + { + /* Create the smblib mutex */ + if(pthread_mutexattr_init(&g_mysql_mutexattr) != 0 || + pthread_mutexattr_settype(&g_mysql_mutexattr, HA_MUTEX_TYPE) || + pthread_mutex_init(&g_mysql_mutex, &g_mysql_mutexattr) != 0) + { + ha_messagex(NULL, LOG_CRIT, "threading problem. can't create mutex"); + return HA_CRITERROR; + } + } + return HA_OK; } @@ -727,6 +772,12 @@ void mysql_destroy(ha_context_t* context) free(ctx->pool); } } + else + { + /* Close the mutex */ + pthread_mutex_destroy(&g_mysql_mutex); + pthread_mutexattr_destroy(&g_mysql_mutexattr); + } bd_destroy(context); ha_messagex(NULL, LOG_INFO, "uninitialized mysql handler"); diff --git a/daemon/ntlm.c b/daemon/ntlm.c index 8e1aa20..fba4624 100644 --- a/daemon/ntlm.c +++ b/daemon/ntlm.c @@ -39,14 +39,14 @@ ntlm_connection_t; /* The main context */ typedef struct ntlm_context { - /* Settings ---------------------------------------------------------- */ + /* Read Only --------------------------------------------------------- */ const char* server; /* Server to authenticate against */ const char* domain; /* NTLM domain to authenticate against */ const char* backup; /* Backup server if primary is down */ int pending_max; /* Maximum number of connections at once */ int pending_timeout; /* Timeout for authentication (in seconds) */ - /* Context ----------------------------------------------------------- */ + /* Require Locking --------------------------------------------------- */ hsh_t* pending; /* Pending connections */ hsh_t* established; /* Established connections */ } @@ -149,19 +149,19 @@ static int putpending(ntlm_context_t* ctx, const void* key, ntlm_connection_t* c ASSERT(ctx && key && conn); ASSERT(conn->handle); - if(!hsh_get(ctx->pending, key)) - { - ha_lock(NULL); + ha_lock(NULL); + if(!hsh_get(ctx->pending, key)) + { if(!hsh_set(ctx->pending, key, (void*)conn)) { free_hash_object(NULL, conn); ha_messagex(NULL, LOG_ERR, "out of memory"); r = -1; } + } - ha_unlock(NULL); - } + ha_unlock(NULL); return r; } @@ -633,7 +633,7 @@ void ntlm_destroy(ha_context_t* context) ntlm_context_t* ctx = (ntlm_context_t*)(context->ctx_data); if(ctx->pending) - hsh_free(ctx->pending); + hsh_free(ctx->pending); if(ctx->established) hsh_free(ctx->established); @@ -721,18 +721,18 @@ int ntlm_process(ha_request_t* rq) { ha_lock(NULL); - /* - * NTLM trusts a connection after it's been authenticated - * so just pass success for those. Note that we do this - * in the absence of a authorization header so that we - * allow connections to be re-authenticated. - */ + /* + * NTLM trusts a connection after it's been authenticated + * so just pass success for those. Note that we do this + * in the absence of a authorization header so that we + * allow connections to be re-authenticated. + */ - if(hsh_get(ctx->established, key) == NTLM_ESTABLISHED) - { - hsh_touch(ctx->established, key); - rq->resp_code = HA_SERVER_OK; - } + if(hsh_get(ctx->established, key) == NTLM_ESTABLISHED) + { + hsh_touch(ctx->established, key); + rq->resp_code = HA_SERVER_OK; + } ha_unlock(NULL); diff --git a/daemon/pgsql.c b/daemon/pgsql.c index 6bdb158..56eebd1 100644 --- a/daemon/pgsql.c +++ b/daemon/pgsql.c @@ -25,7 +25,7 @@ typedef struct pgsql_context /* Base Handler ------------------------------------------------------ */ bd_context_t bd; - /* Settings ---------------------------------------------------------- */ + /* Read Only --------------------------------------------------------- */ const char* host; /* The connection host or path */ const char* port; /* The connection port */ const char* user; /* The pgsql user name */ @@ -39,7 +39,7 @@ typedef struct pgsql_context int pgsql_max; /* Number of open connections allowed */ int pgsql_timeout; /* Maximum amount of time to dedicate to a query */ - /* Context ----------------------------------------------------------- */ + /* Require Locking --------------------------------------------------- */ PGconn** pool; /* Pool of available connections */ int pool_mark; /* Amount of connections allocated */ } @@ -282,29 +282,42 @@ static const char* make_pgsql_connstring(const ha_request_t* rq, pgsql_context_t static PGconn* get_pgsql_connection(const ha_request_t* rq, pgsql_context_t* ctx) { - PGconn* pg; + PGconn* pg = NULL; const char* connstring; - int i; + int i, create = 1; ASSERT(ctx); - for(i = 0; i < ctx->pgsql_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->pgsql_max; i++) { - ha_messagex(rq, LOG_DEBUG, "using cached pgsql connection"); - pg = ctx->pool[i]; - ctx->pool[i] = NULL; - return pg; + /* An open connection in the pool */ + if(ctx->pool[i]) + { + ha_messagex(rq, LOG_DEBUG, "using cached pgsql connection"); + pg = ctx->pool[i]; + ctx->pool[i] = NULL; + break; + } } - } - if(ctx->pool_mark >= ctx->pgsql_max) - { - ha_messagex(rq, LOG_ERR, "too many open pgsql connections"); - return NULL; - } + if(pg == NULL && ctx->pool_mark >= ctx->pgsql_max) + { + ha_messagex(rq, LOG_ERR, "too many open pgsql connections"); + create = 0; + } + + ha_unlock(NULL); + + if(pg != NULL || create == 0) + return pg; connstring = make_pgsql_connstring(rq, ctx); if(!connstring) @@ -326,16 +339,26 @@ static PGconn* get_pgsql_connection(const ha_request_t* rq, pgsql_context_t* ctx return NULL; } - ctx->pool_mark++; - ha_messagex(rq, LOG_DEBUG, "opened new pgsql connection (total %d)", ctx->pool_mark); + ha_lock(NULL); + + ctx->pool_mark++; + ha_messagex(rq, LOG_DEBUG, "opened new pgsql connection (total %d)", ctx->pool_mark); + + ha_unlock(NULL); + return pg; } static void discard_pgsql_connection(const ha_request_t* rq, pgsql_context_t* ctx, PGconn* pg) { PQfinish(pg); - ctx->pool_mark--; - ha_messagex(rq, LOG_DEBUG, "discarding pgsql connection (total %d)", ctx->pool_mark); + + ha_lock(NULL); + + ctx->pool_mark--; + ha_messagex(rq, LOG_DEBUG, "discarding pgsql connection (total %d)", ctx->pool_mark); + + ha_unlock(NULL); } static void save_pgsql_connection(const ha_request_t* rq, pgsql_context_t* ctx, PGconn* pg) @@ -350,17 +373,21 @@ static void save_pgsql_connection(const ha_request_t* rq, pgsql_context_t* ctx, /* Make sure it's worth saving */ if(PQstatus(pg) != CONNECTION_BAD) { - for(i = 0; i < ctx->pgsql_max; i++) - { - /* An open connection in the pool */ - if(!ctx->pool[i]) + ha_lock(NULL); + + for(i = 0; i < ctx->pgsql_max; i++) { - ha_messagex(rq, LOG_DEBUG, "caching pgsql connection for later use"); - ctx->pool[i] = pg; - pg = NULL; - break; + /* An open connection in the pool */ + if(!ctx->pool[i]) + { + ha_messagex(rq, LOG_DEBUG, "caching pgsql connection for later use"); + ctx->pool[i] = pg; + pg = NULL; + break; + } } - } + + ha_unlock(NULL); } if(pg != NULL) diff --git a/daemon/simple.c b/daemon/simple.c index b37922e..5b56138 100644 --- a/daemon/simple.c +++ b/daemon/simple.c @@ -23,7 +23,7 @@ typedef struct simple_context /* Base Handler ------------------------------------------------------ */ bd_context_t bd; - /* Settings ---------------------------------------------------------- */ + /* Read Only --------------------------------------------------------- */ const char* filename; /* The file name with the user names */ } simple_context_t; diff --git a/sample/httpauthd.conf b/sample/httpauthd.conf index 19b168b..9e4b0e1 100644 --- a/sample/httpauthd.conf +++ b/sample/httpauthd.conf @@ -28,7 +28,7 @@ LDAPFilter: (cn=%u) LDAPBase: dc=fam LDAPScope: sub -[PGSQL:UDBpg] +[PGSQL:UDB] Realm: blah DBServer: sean-2 DBUser: testuser @@ -38,7 +38,7 @@ DBQuery: SELECT * from testauth WHERE username = '%u'; # DBPWType: md5 DBHA1Column: pw -[MYSQL:UDB] +[MYSQL:UDBmy] Realm: blah DBServer: var/lib/mysql/mysql.sock DBUser: testuser -- cgit v1.2.3