commit 119cbdd525811908738e8f5d894fe4117cf123a9
Author: John Dennis <jdennis@redhat.com>
Date: Tue Jun 13 08:22:15 2017 -0400
modify cache functions to take request_rec parameter instead of server_rec
The entire point of the cache is to persist state between requests so
conceptually it makes sense the cache functions would receive a
server_rec pointer because the cache is a server level data
structure. However most cache operations occur in the context of a
request. Passing a request_rec to a cache function has the following
advantages:
1) Any logging during a cache operation should be tied to the request.
2) Any need for temporary memory allocation is much easier to handle
with access to the request's memory pool which is cleaned up at the
end of the request as opposed to trying manage memory allocations at
the server level.
3) Any need for access to the server_rec is trivially easy to obtain
from the request_rec via r->server. In fact the caller of cache
functions inside requests simplyy provided the server_rec parameter
via r->server, so why not just pass the request_rec?
These changes are in anticipation of adding enhanced logging and
diagnostics into the cache functions, they will need access to the
request_rec and it's memory pool.
Signed-off-by: John Dennis <jdennis@redhat.com>
diff --git a/auth_mellon.h b/auth_mellon.h
index 78a5f0d..aff658b 100644
--- a/auth_mellon.h
+++ b/auth_mellon.h
@@ -378,23 +378,23 @@ const char *am_cookie_token(request_rec *r);
void am_cache_init(am_mod_cfg_rec *mod_cfg);
-am_cache_entry_t *am_cache_lock(server_rec *s,
+am_cache_entry_t *am_cache_lock(request_rec *r,
am_cache_key_t type, const char *key);
const char *am_cache_entry_get_string(am_cache_entry_t *e,
am_cache_storage_t *slot);
-am_cache_entry_t *am_cache_new(server_rec *s,
+am_cache_entry_t *am_cache_new(request_rec *r,
const char *key,
const char *cookie_token);
-void am_cache_unlock(server_rec *s, am_cache_entry_t *entry);
+void am_cache_unlock(request_rec *r, am_cache_entry_t *entry);
-void am_cache_update_expires(am_cache_entry_t *t, apr_time_t expires);
+void am_cache_update_expires(request_rec *r, am_cache_entry_t *t, apr_time_t expires);
void am_cache_env_populate(request_rec *r, am_cache_entry_t *session);
int am_cache_env_append(am_cache_entry_t *session,
const char *var, const char *val);
const char *am_cache_env_fetch_first(am_cache_entry_t *t,
const char *var);
-void am_cache_delete(server_rec *s, am_cache_entry_t *session);
+void am_cache_delete(request_rec *r, am_cache_entry_t *session);
int am_cache_set_lasso_state(am_cache_entry_t *session,
const char *lasso_identity,
diff --git a/auth_mellon_cache.c b/auth_mellon_cache.c
index 7d51589..af5c267 100644
--- a/auth_mellon_cache.c
+++ b/auth_mellon_cache.c
@@ -70,14 +70,14 @@ void am_cache_init(am_mod_cfg_rec *mod_cfg)
* after you are done with it.
*
* Parameters:
- * server_rec *s The current server.
+ * request_rec *r The request we are processing.
* am_cache_key_t type AM_CACHE_SESSION or AM_CACHE_NAMEID
* const char *key The session key or user
*
* Returns:
* The session entry on success or NULL on failure.
*/
-am_cache_entry_t *am_cache_lock(server_rec *s,
+am_cache_entry_t *am_cache_lock(request_rec *r,
am_cache_key_t type,
const char *key)
{
@@ -104,14 +104,14 @@ am_cache_entry_t *am_cache_lock(server_rec *s,
break;
}
- mod_cfg = am_get_mod_cfg(s);
+ mod_cfg = am_get_mod_cfg(r->server);
/* Lock the table. */
if((rv = apr_global_mutex_lock(mod_cfg->lock)) != APR_SUCCESS) {
- ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
- "apr_global_mutex_lock() failed [%d]: %s",
- rv, apr_strerror(rv, buffer, sizeof(buffer)));
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+ "apr_global_mutex_lock() failed [%d]: %s",
+ rv, apr_strerror(rv, buffer, sizeof(buffer)));
return NULL;
}
@@ -271,7 +271,7 @@ const char *am_cache_entry_get_string(am_cache_entry_t *e,
* Remember to unlock the table with am_cache_unlock(...) afterwards.
*
* Parameters:
- * server_rec *s The current server.
+ * request_rec *r The request we are processing.
* const char *key The key of the session to allocate.
* const char *cookie_token The cookie token to tie the session to.
*
@@ -279,7 +279,7 @@ const char *am_cache_entry_get_string(am_cache_entry_t *e,
* The new session entry on success. NULL if key is a invalid session
* key.
*/
-am_cache_entry_t *am_cache_new(server_rec *s,
+am_cache_entry_t *am_cache_new(request_rec *r,
const char *key,
const char *cookie_token)
{
@@ -298,14 +298,14 @@ am_cache_entry_t *am_cache_new(server_rec *s,
}
- mod_cfg = am_get_mod_cfg(s);
+ mod_cfg = am_get_mod_cfg(r->server);
/* Lock the table. */
if((rv = apr_global_mutex_lock(mod_cfg->lock)) != APR_SUCCESS) {
- ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
- "apr_global_mutex_lock() failed [%d]: %s",
- rv, apr_strerror(rv, buffer, sizeof(buffer)));
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+ "apr_global_mutex_lock() failed [%d]: %s",
+ rv, apr_strerror(rv, buffer, sizeof(buffer)));
return NULL;
}
@@ -357,11 +357,11 @@ am_cache_entry_t *am_cache_new(server_rec *s,
age = (current_time - t->access) / 1000000;
if(age < 3600) {
- ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s,
- "Dropping LRU entry entry with age = %" APR_TIME_T_FMT
- "s, which is less than one hour. It may be a good"
- " idea to increase MellonCacheSize.",
- age);
+ ap_log_rerror(APLOG_MARK, APLOG_NOTICE, 0, r,
+ "Dropping LRU entry entry with age = %" APR_TIME_T_FMT
+ "s, which is less than one hour. It may be a good"
+ " idea to increase MellonCacheSize.",
+ age);
}
}
@@ -393,8 +393,8 @@ am_cache_entry_t *am_cache_new(server_rec *s,
/* For some strange reason our cookie token is too big to fit in the
* session. This should never happen outside of absurd configurations.
*/
- ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
- "Unable to store cookie token in new session.");
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+ "Unable to store cookie token in new session.");
t->key[0] = '\0'; /* Mark the entry as free. */
apr_global_mutex_unlock(mod_cfg->lock);
return NULL;
@@ -407,20 +407,20 @@ am_cache_entry_t *am_cache_new(server_rec *s,
/* This function unlocks a session entry.
*
* Parameters:
- * server_rec *s The current server.
+ * request_rec *r The request we are processing.
* am_cache_entry_t *entry The session entry.
*
* Returns:
* Nothing.
*/
-void am_cache_unlock(server_rec *s, am_cache_entry_t *entry)
+void am_cache_unlock(request_rec *r, am_cache_entry_t *entry)
{
am_mod_cfg_rec *mod_cfg;
/* Update access time. */
entry->access = apr_time_now();
- mod_cfg = am_get_mod_cfg(s);
+ mod_cfg = am_get_mod_cfg(r->server);
apr_global_mutex_unlock(mod_cfg->lock);
}
@@ -429,13 +429,14 @@ void am_cache_unlock(server_rec *s, am_cache_entry_t *entry)
* timestamp is earlier than the previous.
*
* Parameters:
+ * request_rec *r The request we are processing.
* am_cache_entry_t *t The current session.
* apr_time_t expires The new timestamp.
*
* Returns:
* Nothing.
*/
-void am_cache_update_expires(am_cache_entry_t *t, apr_time_t expires)
+void am_cache_update_expires(request_rec *r, am_cache_entry_t *t, apr_time_t expires)
{
/* Check if we should update the expires timestamp. */
if(t->expires == 0 || t->expires > expires) {
@@ -698,13 +699,13 @@ void am_cache_env_populate(request_rec *r, am_cache_entry_t *t)
/* This function deletes a given key from the session store.
*
* Parameters:
- * server_rec *s The current server.
+ * request_rec *r The request we are processing.
* am_cache_entry_t *cache The entry we are deleting.
*
* Returns:
* Nothing.
*/
-void am_cache_delete(server_rec *s, am_cache_entry_t *cache)
+void am_cache_delete(request_rec *r, am_cache_entry_t *cache)
{
/* We write a null-byte at the beginning of the key to
* mark this slot as unused.
@@ -712,7 +713,7 @@ void am_cache_delete(server_rec *s, am_cache_entry_t *cache)
cache->key[0] = '\0';
/* Unlock the entry. */
- am_cache_unlock(s, cache);
+ am_cache_unlock(r, cache);
}
diff --git a/auth_mellon_handler.c b/auth_mellon_handler.c
index 5661083..44a5ee9 100644
--- a/auth_mellon_handler.c
+++ b/auth_mellon_handler.c
@@ -1479,7 +1479,7 @@ static void am_handle_session_expire(request_rec *r, am_cache_entry_t *session,
/* Updates the expires timestamp if this one is earlier than the
* previous timestamp.
*/
- am_cache_update_expires(session, t);
+ am_cache_update_expires(r, session, t);
}
}
@@ -1517,10 +1517,10 @@ static int add_attributes(am_cache_entry_t *session, request_rec *r,
/* Set expires to whatever is set by MellonSessionLength. */
if(dir_cfg->session_length == -1) {
/* -1 means "use default. The current default is 86400 seconds. */
- am_cache_update_expires(session, apr_time_now()
+ am_cache_update_expires(r, session, apr_time_now()
+ apr_time_make(86400, 0));
} else {
- am_cache_update_expires(session, apr_time_now()
+ am_cache_update_expires(r, session, apr_time_now()
+ apr_time_make(dir_cfg->session_length, 0));
}
diff --git a/auth_mellon_session.c b/auth_mellon_session.c
index fca6c01..856dbb6 100644
--- a/auth_mellon_session.c
+++ b/auth_mellon_session.c
@@ -39,7 +39,7 @@ am_cache_entry_t *am_lock_and_validate(request_rec *r,
am_cache_key_t type,
const char *key)
{
- am_cache_entry_t *session = am_cache_lock(r->server, type, key);
+ am_cache_entry_t *session = am_cache_lock(r, type, key);
if (session == NULL) {
return NULL;
}
@@ -54,7 +54,7 @@ am_cache_entry_t *am_lock_and_validate(request_rec *r,
"request has {%s}.",
cookie_token_session,
cookie_token_target);
- am_cache_unlock(r->server, session);
+ am_cache_unlock(r, session);
return NULL;
}
@@ -124,7 +124,7 @@ am_cache_entry_t *am_new_request_session(request_rec *r)
am_cookie_set(r, session_id);
const char *cookie_token = am_cookie_token(r);
- return am_cache_new(r->server, session_id, cookie_token);
+ return am_cache_new(r, session_id, cookie_token);
}
@@ -140,7 +140,7 @@ am_cache_entry_t *am_new_request_session(request_rec *r)
*/
void am_release_request_session(request_rec *r, am_cache_entry_t *session)
{
- am_cache_unlock(r->server, session);
+ am_cache_unlock(r, session);
}
@@ -164,5 +164,5 @@ void am_delete_request_session(request_rec *r, am_cache_entry_t *session)
}
/* Delete session from the session store. */
- am_cache_delete(r->server, session);
+ am_cache_delete(r, session);
}