From e1cb9bc4ee84d72cce4ae77c9f5b972f11ac8a76 Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Sun, 16 Jan 2011 01:43:25 +0100 Subject: [PATCH 01/21] Defaults should stay unchanged --- mod_upload_progress.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index de494d0..95cf063 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -14,8 +14,8 @@ #include #endif -#define PROGRESS_ID "upload_id" -#define PROGRESS_ID_LEN 9 +#define PROGRESS_ID "X-Progress-ID" +#define PROGRESS_ID_LEN 14 #define CACHE_LOCK() do { \ if (config->cache_lock) { \ From f8964144b513d8d81f7c906cc219b17ba10beefa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Sun, 23 Jan 2011 07:10:28 +0100 Subject: [PATCH 02/21] Removed unused ServerConfig field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index 1daa32b..51a4941 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -69,7 +69,6 @@ typedef struct { typedef struct { server_rec *server; - apr_rmm_off_t cache_offset; apr_pool_t *pool; apr_global_mutex_t *cache_lock; char *lock_file; /* filename for shm lock mutex */ @@ -667,7 +666,6 @@ apr_status_t upload_progress_cache_init(apr_pool_t *pool, ServerConfig *config) return 0; } cache->head = NULL; - config->cache_offset = block; config->cache = cache; #endif @@ -756,7 +754,6 @@ int upload_progress_init(apr_pool_t *p, apr_pool_t *plog, st_vhost->cache_shm = config->cache_shm; st_vhost->cache_rmm = config->cache_rmm; st_vhost->cache_file = config->cache_file; - st_vhost->cache_offset = config->cache_offset; st_vhost->cache = config->cache; ap_log_error(APLOG_MARK, APLOG_DEBUG, result, s, "Upload Progress: merging Shared Cache conf: shm=0x%pp rmm=0x%pp " From 715c773e3de319c8498f41a7d444831720b228b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Sun, 23 Jan 2011 09:51:17 +0100 Subject: [PATCH 03/21] Move common code from get_{progress_id,json_callback_param} to separate function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 88 ++++++++++++++++--------------------------- 1 file changed, 32 insertions(+), 56 deletions(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index 51a4941..eebe846 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -298,9 +298,28 @@ static int track_upload_progress(ap_filter_t *f, apr_bucket_brigade *bb, return rv; } +char *get_param_value(char *p, const char *param_name, int *len) { + char pn1[3] = {toupper(param_name[0]), tolower(param_name[0]), 0}; + int pn_len = strlen(param_name); + char *val_end; + static char *param_sep = "&"; + + while (p) { + if ((strncasecmp(p, param_name, pn_len) == 0) && (p[pn_len] == '=')) + break; + if (*p) p++; + p = strpbrk(p, pn1); + } + if (p) { + p += (pn_len + 1); + *len = strcspn(p, param_sep); + } + return p; +} + const char *get_progress_id(request_rec *r) { - char *p, *start_p, *end_p; - int i; + char *val; + int len; /**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "get_progress_id()"); @@ -308,70 +327,27 @@ const char *get_progress_id(request_rec *r) { const char *id = apr_table_get(r->headers_in, PROGRESS_ID); //if not found check args - if(id == NULL) { - if (r->args) { - i = 0; - p = r->args; - do { - int len = strlen(p); - if ((len >= PROGRESS_ID_LEN + strlen("=")) && strncasecmp(p, PROGRESS_ID "=", PROGRESS_ID_LEN + strlen("=") ) == 0) { - i = 1; - break; - } - if (len<=0) - break; - } - while(p++); - - if (i) { - i = 0; - start_p = p += PROGRESS_ID_LEN + strlen("="); - end_p = r->args + strlen(r->args); - while (p <= end_p && *p++ != '&') { - i++; - } - - return apr_pstrndup(r->connection->pool, start_p, p-start_p-1); - } - } + if (id == NULL) { + val = get_param_value(r->args, PROGRESS_ID, &len); + if (val) + id = apr_pstrndup(r->connection->pool, val, len); } return id; } const char *get_json_callback_param(request_rec *r) { - char *p, *start_p, *end_p; - int i; - const char *callback = NULL; + char *val; + int len; /**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "get_json_callback_param()"); - if (r->args) { - i = 0; - p = r->args; - do { - int len = strlen(p); - if (len >= 9 && strncasecmp(p, "callback=", 9) == 0) { - i = 1; - break; - } - if (len<=0) - break; - } - while(p++); - - if (i) { - i = 0; - start_p = p += 9; - end_p = r->args + strlen(r->args); - while (p <= end_p && *p++ != '&') { - i++; - } - return apr_pstrndup(r->connection->pool, start_p, p-start_p-1); - } + val = get_param_value(r->args, PROGRESS_ID, &len); + if (val) { + return apr_pstrndup(r->connection->pool, val, len); + } else { + return NULL; } - - return callback; } void cache_free(ServerConfig *config, const void *ptr) From e4970ab722e441ef63c62ddff5c949c29a5a18a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Sun, 23 Jan 2011 11:01:42 +0100 Subject: [PATCH 04/21] Remove no longer used macro PROGRESS_ID_LEN MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 1 - 1 file changed, 1 deletion(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index eebe846..ffd0610 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -16,7 +16,6 @@ #endif #define PROGRESS_ID "X-Progress-ID" -#define PROGRESS_ID_LEN strlen(PROGRESS_ID) #define CACHE_LOCK() do { \ if (config->cache_lock) { \ From 2eef1b4091bb84fa5c2c0bcea197de1ee526fb01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Sun, 23 Jan 2011 11:06:20 +0100 Subject: [PATCH 05/21] Lock debugging made conditional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index ffd0610..1aa8a8d 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -17,10 +17,18 @@ #define PROGRESS_ID "X-Progress-ID" +#define DEBUG_LOCKING 0 + +#if DEBUG_LOCKING == 1 +# define LOCKDBG(expr) expr +#else +# define LOCKDBG(expr) +#endif + #define CACHE_LOCK() do { \ if (config->cache_lock) { \ char errbuf[200]; \ - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, config->server, "CACHE_LOCK()"); \ + LOCKDBG(ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, config->server, "CACHE_LOCK()")); \ apr_status_t status = apr_global_mutex_lock(config->cache_lock); \ if (status != APR_SUCCESS) { \ ap_log_error(APLOG_MARK, APLOG_CRIT, status, 0, \ @@ -32,7 +40,7 @@ #define CACHE_UNLOCK() do { \ if (config->cache_lock) \ { \ - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, config->server, "CACHE_UNLOCK()"); \ + LOCKDBG(ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, config->server, "CACHE_UNLOCK()")); \ apr_global_mutex_unlock(config->cache_lock); \ } \ } while (0) From d822d94684f48e013aba356b6173ab6b813425df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Sun, 23 Jan 2011 11:09:03 +0100 Subject: [PATCH 06/21] Disable logging in few less important functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index 1aa8a8d..37d9e0c 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -149,7 +149,7 @@ static void upload_progress_register_hooks (apr_pool_t *p) } ServerConfig *get_server_config(request_rec *r) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "get_server_config()"); +/* ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "get_server_config()"); */ return (ServerConfig*)ap_get_module_config(r->server->module_config, &upload_progress_module); } @@ -243,7 +243,7 @@ void * upload_progress_config_create_dir(apr_pool_t *p, char *dirspec) { } void *upload_progress_config_create_server(apr_pool_t *p, server_rec *s) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "upload_progress_config_create_server()"); +/* ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "upload_progress_config_create_server()"); */ ServerConfig *config = (ServerConfig *)apr_pcalloc(p, sizeof(ServerConfig)); config->cache_file = apr_pstrdup(p, "/tmp/upload_progress_cache"); config->cache_bytes = 51200; @@ -254,7 +254,7 @@ void *upload_progress_config_create_server(apr_pool_t *p, server_rec *s) { int read_request_status(request_rec *r) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "read_request_status()"); +/* ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "read_request_status()"); */ int status; if (r) { @@ -328,7 +328,7 @@ const char *get_progress_id(request_rec *r) { char *val; int len; -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "get_progress_id()"); +/* ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "get_progress_id()"); */ //try to find progress id in headers const char *id = apr_table_get(r->headers_in, PROGRESS_ID); @@ -347,7 +347,7 @@ const char *get_json_callback_param(request_rec *r) { char *val; int len; -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "get_json_callback_param()"); +/* ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "get_json_callback_param()"); */ val = get_param_value(r->args, PROGRESS_ID, &len); if (val) { From 59fa15a2c3df257659ad84f76913fd6c5f512c21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Sun, 23 Jan 2011 11:11:23 +0100 Subject: [PATCH 07/21] Improved log message in reportuploads_handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index 37d9e0c..e795669 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -780,12 +780,12 @@ static int reportuploads_handler(request_rec *r) const char *id = get_progress_id(r); if (id == NULL) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, - "Upload Progress: Not found id in location with reports enabled. uri=%s", id, r->uri); + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, + "Upload Progress: Request without id in location with reports enabled. uri=%s", id, r->uri); return HTTP_NOT_FOUND; } else { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, - "Upload Progress: Found id=%s in location with reports enables. uri=%s", id, r->uri); + "Upload Progress: Request with id=%s in location with reports enabled. uri=%s", id, r->uri); } ServerConfig *config = (ServerConfig*)ap_get_module_config(r->server->module_config, &upload_progress_module); From 0aeed02dfe3f87a9feea8c96b1219d7b033d6b32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Sun, 23 Jan 2011 11:58:59 +0100 Subject: [PATCH 08/21] Allow setting cache filename via -D preprocessor parameter When invoking make with CFLAGS parameter triple quoting is needed: make CFLAGS='"-DCACHE_FILENAME=\\\"/var/cache/apache2/upload_progress_cache\\\""' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index e795669..da60659 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -16,6 +16,9 @@ #endif #define PROGRESS_ID "X-Progress-ID" +#ifndef CACHE_FILENAME +# define CACHE_FILENAME "/tmp/upload_progress_cache" +#endif #define DEBUG_LOCKING 0 @@ -245,7 +248,7 @@ void * upload_progress_config_create_dir(apr_pool_t *p, char *dirspec) { void *upload_progress_config_create_server(apr_pool_t *p, server_rec *s) { /* ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "upload_progress_config_create_server()"); */ ServerConfig *config = (ServerConfig *)apr_pcalloc(p, sizeof(ServerConfig)); - config->cache_file = apr_pstrdup(p, "/tmp/upload_progress_cache"); + config->cache_file = apr_pstrdup(p, CACHE_FILENAME); config->cache_bytes = 51200; apr_pool_create(&config->pool, p); config->server = s; From d4369bb0e9d65813be6ec678d8567674dbff98e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Sun, 23 Jan 2011 12:08:06 +0100 Subject: [PATCH 09/21] Also allow passing PROGRESS_ID nad DEBUG_LOCKING as -D preprocessor parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index da60659..4bddccc 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -15,12 +15,15 @@ #include #endif -#define PROGRESS_ID "X-Progress-ID" +#ifndef PROGRESS_ID +# define PROGRESS_ID "X-Progress-ID" +#endif #ifndef CACHE_FILENAME # define CACHE_FILENAME "/tmp/upload_progress_cache" #endif - -#define DEBUG_LOCKING 0 +#ifndef DEBUG_LOCKING +# define DEBUG_LOCKING 0 +#endif #if DEBUG_LOCKING == 1 # define LOCKDBG(expr) expr From 8e2a3ffc657f0a09fb35fc5e3cb38e55065372b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Sun, 23 Jan 2011 12:10:23 +0100 Subject: [PATCH 10/21] Makefile simplification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- Makefile | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Makefile b/Makefile index ab75a50..30844ab 100644 --- a/Makefile +++ b/Makefile @@ -1,11 +1,7 @@ -include local-options.mk ifndef APXS - APXS := $(shell which apxs2) -endif - -ifndef APXS - APXS := $(shell which apxs) + APXS := $(shell which apxs2 || which apxs) endif ifndef APXS From 4a639762ee2b52a3b3a0bab29abf8efbfbd7151b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Mon, 24 Jan 2011 00:17:26 +0100 Subject: [PATCH 11/21] Fix json callback parameter reading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index 4bddccc..d1f19f0 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -18,6 +18,9 @@ #ifndef PROGRESS_ID # define PROGRESS_ID "X-Progress-ID" #endif +#ifndef JSON_CB_PARAM +# define JSON_CB_PARAM "callback" +#endif #ifndef CACHE_FILENAME # define CACHE_FILENAME "/tmp/upload_progress_cache" #endif @@ -355,7 +358,7 @@ const char *get_json_callback_param(request_rec *r) { /* ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "get_json_callback_param()"); */ - val = get_param_value(r->args, PROGRESS_ID, &len); + val = get_param_value(r->args, JSON_CB_PARAM, &len); if (val) { return apr_pstrndup(r->connection->pool, val, len); } else { From 61eacdb48a85b1459aa8dd4decf0041c97f4fb14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Mon, 24 Jan 2011 01:10:30 +0100 Subject: [PATCH 12/21] Prevent shared memory exhaustion by clients who set very long progress id keys Key is stored in fixed size char array inside upload_progress_node struct. Note: key in struct may not be null terminated - use strn* functions to process. Minor drawback: longer keys are truncated for comparison, maximum key size is set during compilation. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index d1f19f0..51a4883 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -27,6 +27,10 @@ #ifndef DEBUG_LOCKING # define DEBUG_LOCKING 0 #endif +#define PROGRESS_KEY_TEMPLATE "12345678-0000-0000-0000-123456789012" +#ifndef PROGRESS_KEY_LEN +# define PROGRESS_KEY_LEN strlen(PROGRESS_KEY_TEMPLATE) +#endif #if DEBUG_LOCKING == 1 # define LOCKDBG(expr) expr @@ -61,16 +65,16 @@ typedef struct { typedef struct upload_progress_node_s{ - int done; apr_size_t length; apr_size_t received; int err_status; - char *key; time_t started_at; apr_size_t speed; /* bytes per second */ time_t expires; struct upload_progress_node_s* next; struct upload_progress_node_s* prev; + int done; + char key[PROGRESS_KEY_LEN]; }upload_progress_node_t; typedef struct { @@ -346,6 +350,7 @@ const char *get_progress_id(request_rec *r) { if (id == NULL) { val = get_param_value(r->args, PROGRESS_ID, &len); if (val) + if (len > PROGRESS_KEY_LEN) len = PROGRESS_KEY_LEN; id = apr_pstrndup(r->connection->pool, val, len); } @@ -380,15 +385,9 @@ void cache_free(ServerConfig *config, const void *ptr) } } -char *fetch_key(ServerConfig *config, char *key) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "fetch_key()"); - return (char *)apr_rmm_addr_get(config->cache_rmm, apr_rmm_offset_get(config->cache_rmm, key)); -} - int check_node(ServerConfig *config, upload_progress_node_t *node, const char *key) { /**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "check_node()"); - char *node_key = fetch_key(config, node->key); - return strcasecmp(node_key, key) == 0 ? 1 : 0; + return strncasecmp(node->key, key, PROGRESS_KEY_LEN) == 0 ? 1 : 0; } upload_progress_node_t *fetch_node(ServerConfig *config, upload_progress_node_t *node) { @@ -440,19 +439,8 @@ upload_progress_node_t *store_node(ServerConfig *config, const char *key) { /**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "store_node()"); node = block ? (upload_progress_node_t *)apr_rmm_addr_get(config->cache_rmm, block) : NULL; - if (node == NULL) { - return NULL; - } - node->next = NULL; - - block = apr_rmm_calloc(config->cache_rmm, strlen(key)+1); - node->key = block ? (char *)apr_rmm_addr_get(config->cache_rmm, block) : NULL; - if (node->key != NULL) { - sprintf(node->key, "%s\0", key); - } else { - apr_rmm_free(config->cache_rmm, apr_rmm_offset_get(config->cache_rmm, (void *)node)); - node = NULL; - } + if (node) + strncpy(node->key, key, PROGRESS_KEY_LEN); return node; } @@ -560,12 +548,10 @@ static void clean_old_connections(request_rec *r) { /* head */ upload_progress_cache_t *cache = fetch_cache(config); cache->head = fetch_node(config, node->next); - cache_free(config, node->key); cache_free(config, node); node = cache->head; } else { prev->next = node->next; - cache_free(config, node->key); cache_free(config, node); node = prev; } From 1647997f57fdded9b0f61c44c957f7c8e28c3612 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Mon, 24 Jan 2011 04:01:19 +0100 Subject: [PATCH 13/21] Move setting vhost server config field to proper place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index 51a4883..dc978db 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -739,6 +739,7 @@ int upload_progress_init(apr_pool_t *p, apr_pool_t *plog, s_vhost->server_hostname); #endif + st_vhost->cache_lock = config->cache_lock; st_vhost->lock_file = config->lock_file; s_vhost = s_vhost->next; } @@ -860,8 +861,6 @@ static void upload_progress_child_init(apr_pool_t *p, server_rec *s) apr_status_t sts; ServerConfig *st = (ServerConfig *)ap_get_module_config(s->module_config, &upload_progress_module); - server_rec *s_vhost; - ServerConfig *st_vhost; if (!st->cache_lock) return; @@ -873,11 +872,4 @@ static void upload_progress_child_init(apr_pool_t *p, server_rec *s) APR_PID_T_FMT ".", st->lock_file, getpid()); } - - s_vhost = s->next; - while (s_vhost) { - st_vhost = (ServerConfig *)ap_get_module_config(s_vhost->module_config, &upload_progress_module); - st_vhost->cache_lock = st->cache_lock; - s_vhost = s_vhost->next; - } } From 525ab0ffb356f2d916a2357b1f4b79841be54480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Mon, 24 Jan 2011 04:08:16 +0100 Subject: [PATCH 14/21] Add logging of missing global mutex on child process init MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index dc978db..6e19ab5 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -862,8 +862,10 @@ static void upload_progress_child_init(apr_pool_t *p, server_rec *s) apr_status_t sts; ServerConfig *st = (ServerConfig *)ap_get_module_config(s->module_config, &upload_progress_module); - if (!st->cache_lock) + if (!st->cache_lock) { + ap_log_error(APLOG_MARK, APLOG_CRIT, 0, s, "Global mutex not set."); return; + } sts = apr_global_mutex_child_init(&st->cache_lock, st->lock_file, p); if (sts != APR_SUCCESS) { From 582804e5f2b0053b311d9a16bd34186f47a7dbc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Mon, 24 Jan 2011 09:35:52 +0100 Subject: [PATCH 15/21] Do not explicitly destroy shared memory on module unload Apache pool management routines should take care of it automatically. This should eliminate following error that sometimes show in my error.log "seg fault or similar nasty error detected in the parent process" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index 6e19ab5..44feb5e 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -584,22 +584,6 @@ static apr_status_t upload_progress_cache_module_kill(void *data) { /**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_cache_module_kill()"); - ServerConfig *st = (ServerConfig*)data; - - upload_progress_destroy_cache(st); - -#if APR_HAS_SHARED_MEMORY - //FIXME!: Second block of code gets never executed! - if (st->cache_rmm != NULL) { - apr_rmm_destroy (st->cache_rmm); - st->cache_rmm = NULL; - } - if (st->cache_shm != NULL) { - apr_status_t result = apr_shm_destroy(st->cache_shm); - st->cache_shm = NULL; - return result; - } -#endif return APR_SUCCESS; } From 2395a2fba1472a6348ed4a09bcf1ec2eeaab2aaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Mon, 24 Jan 2011 09:45:31 +0100 Subject: [PATCH 16/21] Simplify progress nodes memory organization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When id key data is stored directly in node and shared memory size is known, all data structures can be allocated upfront. Data is organized as: cache->count: size of arrays, derived from available shared memory size, cache->nodes: array of nodes (static size), cache->list: array of node indexes 1..count (static size), cache->active: how many nodes are used by pending uploads Active node indexes are stored on the beginning of the list. Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 214 ++++++++++++------------------------------ 1 file changed, 62 insertions(+), 152 deletions(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index 44feb5e..c1793e5 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -71,14 +71,15 @@ typedef struct upload_progress_node_s{ time_t started_at; apr_size_t speed; /* bytes per second */ time_t expires; - struct upload_progress_node_s* next; - struct upload_progress_node_s* prev; int done; char key[PROGRESS_KEY_LEN]; }upload_progress_node_t; typedef struct { - upload_progress_node_t *head; /* keep head of the list */ + int count; + int active; + upload_progress_node_t *nodes; /* all nodes allocated at once */ + int *list; /* static array of node indexes, list begins with indexes of active nodes */ }upload_progress_cache_t; typedef struct { @@ -371,80 +372,10 @@ const char *get_json_callback_param(request_rec *r) { } } -void cache_free(ServerConfig *config, const void *ptr) -{ -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "cache_free()"); - if (config->cache_rmm) { - if (ptr) - /* Free in shared memory */ - apr_rmm_free(config->cache_rmm, apr_rmm_offset_get(config->cache_rmm, (void *)ptr)); - } else { - if (ptr) - /* Cache shm is not used */ - free((void *)ptr); - } -} - -int check_node(ServerConfig *config, upload_progress_node_t *node, const char *key) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "check_node()"); +int check_node(upload_progress_node_t *node, const char *key) { return strncasecmp(node->key, key, PROGRESS_KEY_LEN) == 0 ? 1 : 0; } -upload_progress_node_t *fetch_node(ServerConfig *config, upload_progress_node_t *node) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "fetch_node()"); - return (upload_progress_node_t *)apr_rmm_addr_get(config->cache_rmm, apr_rmm_offset_get(config->cache_rmm, node)); -} - -upload_progress_cache_t *fetch_cache(ServerConfig *config) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "fetch_cache()"); - return (upload_progress_cache_t *)apr_rmm_addr_get(config->cache_rmm, apr_rmm_offset_get(config->cache_rmm, config->cache)); -} - -upload_progress_node_t *fetch_first_node(ServerConfig *config) { - upload_progress_cache_t *cache; - -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "fetch_first_node()"); - - cache = fetch_cache(config); - if(cache->head == NULL) { - return NULL; - } - - return fetch_node(config, cache->head); -} - -upload_progress_node_t *fetch_last_node(ServerConfig *config) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "fetch_last_node()"); - - upload_progress_cache_t *cache; - upload_progress_node_t *node; - - cache = fetch_cache(config); - if(cache->head == NULL) { - return NULL; - } - - node = fetch_node(config, cache->head); - while(node->next != NULL) { - node = fetch_node(config, node->next); - } - - return node; -} - -upload_progress_node_t *store_node(ServerConfig *config, const char *key) { - apr_rmm_off_t block = apr_rmm_calloc(config->cache_rmm, sizeof(upload_progress_node_t)); - upload_progress_node_t *node; - -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "store_node()"); - - node = block ? (upload_progress_node_t *)apr_rmm_addr_get(config->cache_rmm, block) : NULL; - if (node) - strncpy(node->key, key, PROGRESS_KEY_LEN); - - return node; -} - void fill_new_upload_node_data(upload_progress_node_t *node, request_rec *r) { const char *content_length; @@ -466,56 +397,39 @@ void fill_new_upload_node_data(upload_progress_node_t *node, request_rec *r) { upload_progress_node_t* insert_node(request_rec *r, const char *key) { /**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "insert_node()"); - upload_progress_node_t *node; - upload_progress_cache_t *cache; - ServerConfig *config = (ServerConfig*)ap_get_module_config(r->server->module_config, &upload_progress_module); + upload_progress_cache_t *cache = config->cache; + upload_progress_node_t *node; - upload_progress_node_t *head = fetch_first_node(config); - node = store_node(config, key); - if (node == NULL) + if (cache->active == cache->count) { + ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, r->server, "Cache full"); return NULL; - - if (head == NULL) { - /* list is empty */ - cache = fetch_cache(config); - cache->head = node; - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, - "Upload Progress: Inserted node into an empty list."); - } else { - upload_progress_node_t *tail = fetch_last_node(config); - tail->next = node; - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, - "Upload Progress: Inserted node at the end of the list."); } + node = &cache->nodes[cache->list[cache->active]]; + cache->active += 1; + strncpy(node->key, key, PROGRESS_KEY_LEN); fill_new_upload_node_data(node, r); - node->next = NULL; return node; } upload_progress_node_t *find_node(request_rec *r, const char *key) { - upload_progress_node_t *node; - /**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "find_node()"); ServerConfig *config = (ServerConfig*)ap_get_module_config(r->server->module_config, &upload_progress_module); + upload_progress_cache_t *cache = config->cache; + upload_progress_node_t *node, *nodes = cache->nodes; + int *list = cache->list; + int active = cache->active; + int i; - node = fetch_first_node(config); - if(node == NULL) { - return NULL; + for (i = 0; i < active; i++) { + node = &nodes[list[i]]; + if (check_node(node, key)) + return node; } - - while(node != NULL) { - if(check_node(config, node, key)) { - return node; - } - - node = fetch_node(config, node->next); - } - - return node; + return NULL; } static apr_status_t upload_progress_cleanup(void *data) @@ -537,49 +451,26 @@ static apr_status_t upload_progress_cleanup(void *data) static void clean_old_connections(request_rec *r) { /**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "clean_old_connections()"); - upload_progress_node_t *prev = NULL; ServerConfig *config = get_server_config(r); - - upload_progress_node_t *node = fetch_first_node(config); - while(node != NULL) { - if(time(NULL) > node->expires && node->done == 1 && node->expires != -1) { - /*clean*/ - if(prev == NULL) { - /* head */ - upload_progress_cache_t *cache = fetch_cache(config); - cache->head = fetch_node(config, node->next); - cache_free(config, node); - node = cache->head; - } else { - prev->next = node->next; - cache_free(config, node); - node = prev; - } - continue; + upload_progress_cache_t *cache = config->cache; + upload_progress_node_t *node, *nodes = cache->nodes; + int *list = cache->list; + int i, tmp; + time_t t = time(NULL); + + for (i = 0; i < cache->active; i++) { + node = &nodes[list[i]]; + if (t > node->expires && node->done == 1 && node->expires != -1) { + cache->active -= 1; + tmp = list[cache->active]; + list[cache->active] = list[i]; + list[i] = tmp; + i--; } - prev = node; - node = fetch_node(config, node->next); } } -void upload_progress_destroy_cache(ServerConfig *config) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_destroy_cache()"); - - upload_progress_cache_t *cache = fetch_cache(config); - upload_progress_node_t *node, *temp; - - node = fetch_node(config, cache->head); - while(node != NULL) { - temp = fetch_node(config, node->next); - - cache_free(config, node); - node = temp; - } - - cache_free(config, cache); -} - static apr_status_t upload_progress_cache_module_kill(void *data) { /**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_cache_module_kill()"); @@ -587,6 +478,12 @@ static apr_status_t upload_progress_cache_module_kill(void *data) return APR_SUCCESS; } +void *rmm_calloc(apr_rmm_t *rmm, apr_size_t reqsize) +{ + apr_rmm_off_t block = apr_rmm_calloc(rmm, reqsize); + return block ? apr_rmm_addr_get(rmm, block) : NULL; +} + apr_status_t upload_progress_cache_init(apr_pool_t *pool, ServerConfig *config) { /**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_cache_init()"); @@ -595,7 +492,7 @@ apr_status_t upload_progress_cache_init(apr_pool_t *pool, ServerConfig *config) apr_status_t result; apr_size_t size; upload_progress_cache_t *cache; - apr_rmm_off_t block; + int nodes_cnt, i; if (config->cache_file) { /* Remove any existing shm segment with this name. */ @@ -622,13 +519,26 @@ apr_status_t upload_progress_cache_init(apr_pool_t *pool, ServerConfig *config) apr_pool_cleanup_register(config->pool, config , upload_progress_cache_module_kill, apr_pool_cleanup_null); /* init cache object */ - block = apr_rmm_calloc(config->cache_rmm, sizeof(upload_progress_cache_t)); - cache = block ? (upload_progress_cache_t *)apr_rmm_addr_get(config->cache_rmm, block) : NULL; - if (cache == NULL) { - return 0; - } - cache->head = NULL; + cache = (upload_progress_cache_t *)rmm_calloc(config->cache_rmm, + sizeof(upload_progress_cache_t)); + if (!cache) return APR_ENOMEM; + config->cache = cache; + nodes_cnt = ((size - sizeof(upload_progress_cache_t)) / + (sizeof(upload_progress_node_t) + sizeof(int))) - 1; + cache->count = nodes_cnt; + cache->active = 0; + + cache->list = (int *)rmm_calloc(config->cache_rmm, nodes_cnt * sizeof(int)); + if (!cache->list) return APR_ENOMEM; + for (i = 0; i < nodes_cnt; i++) cache->list[i] = i; + + cache->nodes = (upload_progress_node_t *)rmm_calloc(config->cache_rmm, + nodes_cnt * sizeof(upload_progress_node_t)); + if (!cache->nodes) return APR_ENOMEM; + + ap_log_error(APLOG_MARK, APLOG_INFO, 0, global_server, + "Upload Progress: monitoring %i simultaneous uploads", nodes_cnt); #endif return APR_SUCCESS; From 96c4ce8b655deba6dd44315db24b12e7db6b0f6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Mon, 24 Jan 2011 10:27:46 +0100 Subject: [PATCH 17/21] Make most debugging noise optional, pass -DUP_DEBUG=1 in CFLAGS to reenable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 74 +++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 41 deletions(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index c1793e5..e7e25fb 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -24,24 +24,24 @@ #ifndef CACHE_FILENAME # define CACHE_FILENAME "/tmp/upload_progress_cache" #endif -#ifndef DEBUG_LOCKING -# define DEBUG_LOCKING 0 +#ifndef UP_DEBUG +# define UP_DEBUG 0 #endif #define PROGRESS_KEY_TEMPLATE "12345678-0000-0000-0000-123456789012" #ifndef PROGRESS_KEY_LEN # define PROGRESS_KEY_LEN strlen(PROGRESS_KEY_TEMPLATE) #endif -#if DEBUG_LOCKING == 1 -# define LOCKDBG(expr) expr +#if UP_DEBUG == 1 +# define up_log(...) ap_log_error( __VA_ARGS__ ) #else -# define LOCKDBG(expr) +# define up_log(...) #endif #define CACHE_LOCK() do { \ if (config->cache_lock) { \ char errbuf[200]; \ - LOCKDBG(ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, config->server, "CACHE_LOCK()")); \ + up_log(APLOG_MARK, APLOG_DEBUG, 0, config->server, "CACHE_LOCK()"); \ apr_status_t status = apr_global_mutex_lock(config->cache_lock); \ if (status != APR_SUCCESS) { \ ap_log_error(APLOG_MARK, APLOG_CRIT, status, 0, \ @@ -53,7 +53,7 @@ #define CACHE_UNLOCK() do { \ if (config->cache_lock) \ { \ - LOCKDBG(ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, config->server, "CACHE_UNLOCK()")); \ + up_log(APLOG_MARK, APLOG_DEBUG, 0, config->server, "CACHE_UNLOCK()"); \ apr_global_mutex_unlock(config->cache_lock); \ } \ } while (0) @@ -153,7 +153,7 @@ module AP_MODULE_DECLARE_DATA upload_progress_module = static void upload_progress_register_hooks (apr_pool_t *p) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_register_hooks()"); +/**/up_log(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_register_hooks()"); ap_hook_fixups(upload_progress_handle_request, NULL, NULL, APR_HOOK_FIRST); ap_hook_handler(reportuploads_handler, NULL, NULL, APR_HOOK_FIRST); @@ -163,13 +163,12 @@ static void upload_progress_register_hooks (apr_pool_t *p) } ServerConfig *get_server_config(request_rec *r) { -/* ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "get_server_config()"); */ return (ServerConfig*)ap_get_module_config(r->server->module_config, &upload_progress_module); } static int upload_progress_handle_request(request_rec *r) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "upload_progress_handle_request()"); +/**/up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "upload_progress_handle_request()"); DirConfig* dir = (DirConfig*)ap_get_module_config(r->per_dir_config, &upload_progress_module); ServerConfig *config = get_server_config(r); @@ -182,7 +181,7 @@ static int upload_progress_handle_request(request_rec *r) const char* id = get_progress_id(r); if (id != NULL) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, + up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "Upload Progress: Progress id found: %s.", id); CACHE_LOCK(); @@ -191,11 +190,11 @@ static int upload_progress_handle_request(request_rec *r) if (node == NULL) { node = insert_node(r, id); if (node) - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, + up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "Upload Progress: Added upload with id=%s to list.", id); } else if (node->done) { fill_new_upload_node_data(node, r); - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, + up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "Upload Progress: Reused existing node with id '%s'.", id); } else { node = NULL; @@ -218,7 +217,7 @@ static int upload_progress_handle_request(request_rec *r) static const char *report_upload_progress_cmd(cmd_parms *cmd, void *config, int arg) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "report_upload_progress_cmd()"); +/**/up_log(APLOG_MARK, APLOG_DEBUG, 0, global_server, "report_upload_progress_cmd()"); DirConfig* dir = (DirConfig*)config ; dir->report_enabled = arg; return NULL; @@ -226,7 +225,7 @@ static const char *report_upload_progress_cmd(cmd_parms *cmd, void *config, int static const char *track_upload_progress_cmd(cmd_parms *cmd, void *config, int arg) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "track_upload_progress_cmd()"); +/**/up_log(APLOG_MARK, APLOG_DEBUG, 0, global_server, "track_upload_progress_cmd()"); DirConfig* dir = (DirConfig*)config ; dir->track_enabled = arg; return NULL; @@ -234,7 +233,7 @@ static const char *track_upload_progress_cmd(cmd_parms *cmd, void *config, int a static const char* upload_progress_shared_memory_size_cmd(cmd_parms *cmd, void *dummy, const char *arg) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_shared_memory_size_cmd()"); +/**/up_log(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_shared_memory_size_cmd()"); ServerConfig *config = (ServerConfig*)ap_get_module_config(cmd->server->module_config, &upload_progress_module); long long int n = atoi(arg); @@ -249,7 +248,7 @@ static const char* upload_progress_shared_memory_size_cmd(cmd_parms *cmd, void * } void * upload_progress_config_create_dir(apr_pool_t *p, char *dirspec) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_config_create_dir()"); +/**/up_log(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_config_create_dir()"); DirConfig* dir = (DirConfig*)apr_pcalloc(p, sizeof(DirConfig)); dir->report_enabled = 0; dir->track_enabled = 0; @@ -257,7 +256,7 @@ void * upload_progress_config_create_dir(apr_pool_t *p, char *dirspec) { } void *upload_progress_config_create_server(apr_pool_t *p, server_rec *s) { -/* ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "upload_progress_config_create_server()"); */ + up_log(APLOG_MARK, APLOG_DEBUG, 0, s, "upload_progress_config_create_server()"); ServerConfig *config = (ServerConfig *)apr_pcalloc(p, sizeof(ServerConfig)); config->cache_file = apr_pstrdup(p, CACHE_FILENAME); config->cache_bytes = 51200; @@ -268,7 +267,6 @@ void *upload_progress_config_create_server(apr_pool_t *p, server_rec *s) { int read_request_status(request_rec *r) { -/* ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "read_request_status()"); */ int status; if (r) { @@ -287,7 +285,7 @@ static int track_upload_progress(ap_filter_t *f, apr_bucket_brigade *bb, ap_input_mode_t mode, apr_read_type_e block, apr_off_t readbytes) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "track_upload_progress()"); +/**/up_log(APLOG_MARK, APLOG_DEBUG, 0, global_server, "track_upload_progress()"); apr_status_t rv; upload_progress_node_t *node; ServerConfig* config = get_server_config(f->r); @@ -342,8 +340,6 @@ const char *get_progress_id(request_rec *r) { char *val; int len; -/* ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "get_progress_id()"); */ - //try to find progress id in headers const char *id = apr_table_get(r->headers_in, PROGRESS_ID); @@ -362,8 +358,6 @@ const char *get_json_callback_param(request_rec *r) { char *val; int len; -/* ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "get_json_callback_param()"); */ - val = get_param_value(r->args, JSON_CB_PARAM, &len); if (val) { return apr_pstrndup(r->connection->pool, val, len); @@ -379,8 +373,6 @@ int check_node(upload_progress_node_t *node, const char *key) { void fill_new_upload_node_data(upload_progress_node_t *node, request_rec *r) { const char *content_length; -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "fill_new_upload_node_data()"); - node->received = 0; node->done = 0; node->err_status = 0; @@ -395,7 +387,7 @@ void fill_new_upload_node_data(upload_progress_node_t *node, request_rec *r) { } upload_progress_node_t* insert_node(request_rec *r, const char *key) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "insert_node()"); +/**/up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "insert_node()"); ServerConfig *config = (ServerConfig*)ap_get_module_config(r->server->module_config, &upload_progress_module); upload_progress_cache_t *cache = config->cache; @@ -415,7 +407,7 @@ upload_progress_node_t* insert_node(request_rec *r, const char *key) { } upload_progress_node_t *find_node(request_rec *r, const char *key) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "find_node()"); +/**/up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "find_node()"); ServerConfig *config = (ServerConfig*)ap_get_module_config(r->server->module_config, &upload_progress_module); upload_progress_cache_t *cache = config->cache; @@ -434,7 +426,7 @@ upload_progress_node_t *find_node(request_rec *r, const char *key) { static apr_status_t upload_progress_cleanup(void *data) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_cleanup()"); +/**/up_log(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_cleanup()"); /* FIXME: this function should use locking because it modifies node data */ upload_progress_context_t *ctx = (upload_progress_context_t *)data; @@ -449,7 +441,7 @@ static apr_status_t upload_progress_cleanup(void *data) } static void clean_old_connections(request_rec *r) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "clean_old_connections()"); +/**/up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "clean_old_connections()"); ServerConfig *config = get_server_config(r); upload_progress_cache_t *cache = config->cache; @@ -473,7 +465,7 @@ static void clean_old_connections(request_rec *r) { static apr_status_t upload_progress_cache_module_kill(void *data) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_cache_module_kill()"); +/**/up_log(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_cache_module_kill()"); return APR_SUCCESS; } @@ -486,7 +478,7 @@ void *rmm_calloc(apr_rmm_t *rmm, apr_size_t reqsize) apr_status_t upload_progress_cache_init(apr_pool_t *pool, ServerConfig *config) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_cache_init()"); +/**/up_log(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_cache_init()"); #if APR_HAS_SHARED_MEMORY apr_status_t result; @@ -547,7 +539,7 @@ apr_status_t upload_progress_cache_init(apr_pool_t *pool, ServerConfig *config) int upload_progress_init(apr_pool_t *p, apr_pool_t *plog, apr_pool_t *ptemp, server_rec *s) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "upload_progress_init()"); +/**/up_log(APLOG_MARK, APLOG_DEBUG, 0, s, "upload_progress_init()"); /**/global_server = s; apr_status_t result; @@ -627,7 +619,7 @@ int upload_progress_init(apr_pool_t *p, apr_pool_t *plog, st_vhost->cache_rmm = config->cache_rmm; st_vhost->cache_file = config->cache_file; st_vhost->cache = config->cache; - ap_log_error(APLOG_MARK, APLOG_DEBUG, result, s, + up_log(APLOG_MARK, APLOG_DEBUG, result, s, "Upload Progress: merging Shared Cache conf: shm=0x%pp rmm=0x%pp " "for VHOST: %s", config->cache_shm, config->cache_rmm, s_vhost->server_hostname); @@ -651,7 +643,7 @@ int upload_progress_init(apr_pool_t *p, apr_pool_t *plog, static int reportuploads_handler(request_rec *r) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "reportuploads_handler()"); +/**/up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "reportuploads_handler()"); apr_size_t length, received, speed; time_t started_at=0; @@ -670,11 +662,11 @@ static int reportuploads_handler(request_rec *r) const char *id = get_progress_id(r); if (id == NULL) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, + up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "Upload Progress: Request without id in location with reports enabled. uri=%s", id, r->uri); return HTTP_NOT_FOUND; } else { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, + up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "Upload Progress: Request with id=%s in location with reports enabled. uri=%s", id, r->uri); } @@ -689,7 +681,7 @@ static int reportuploads_handler(request_rec *r) CACHE_LOCK(); upload_progress_node_t *node = find_node(r, id); if (node != NULL) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, + up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "Node with id=%s found for report", id); received = node->received; length = node->length; @@ -699,7 +691,7 @@ static int reportuploads_handler(request_rec *r) err_status = node->err_status; found = 1; } else { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, + up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "Node with id=%s not found for report", id); } CACHE_UNLOCK(); @@ -734,7 +726,7 @@ static int reportuploads_handler(request_rec *r) /* get the jsonp callback if any */ const char *jsonp = get_json_callback_param(r); - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, + up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "Upload Progress: JSON-P callback: %s.", jsonp); // fix up response for jsonp request, if needed @@ -751,7 +743,7 @@ static int reportuploads_handler(request_rec *r) static void upload_progress_child_init(apr_pool_t *p, server_rec *s) { -/**/ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "upload_progress_child_init()"); +/**/up_log(APLOG_MARK, APLOG_DEBUG, 0, s, "upload_progress_child_init()"); apr_status_t sts; ServerConfig *st = (ServerConfig *)ap_get_module_config(s->module_config, &upload_progress_module); From eb2cdb6f842947d41e0f1cf0d33c84e56fa94090 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Mon, 24 Jan 2011 10:35:19 +0100 Subject: [PATCH 18/21] Remove uncessary cache_rmm check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index e7e25fb..b66059d 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -672,12 +672,6 @@ static int reportuploads_handler(request_rec *r) ServerConfig *config = (ServerConfig*)ap_get_module_config(r->server->module_config, &upload_progress_module); - if (config->cache_rmm == NULL) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, - "Upload Progress: Cache error while generating report"); - return HTTP_INTERNAL_SERVER_ERROR ; - } - CACHE_LOCK(); upload_progress_node_t *node = find_node(r, id); if (node != NULL) { From 00930ea2651958c0118bb5b19c07b44cc821efe3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Mon, 24 Jan 2011 10:45:21 +0100 Subject: [PATCH 19/21] Module will not work without shared memory, make APR_HAS_SHARED_MEMORY mandatory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index b66059d..5656f26 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -9,6 +9,8 @@ #if APR_HAS_SHARED_MEMORY #include "apr_rmm.h" #include "apr_shm.h" +#else +#error "APR_HAS_SHARED_MEMORY required for upload_progress module" #endif #if APR_HAVE_UNISTD_H @@ -94,11 +96,8 @@ typedef struct { apr_global_mutex_t *cache_lock; char *lock_file; /* filename for shm lock mutex */ apr_size_t cache_bytes; - -#if APR_HAS_SHARED_MEMORY apr_shm_t *cache_shm; apr_rmm_t *cache_rmm; -#endif char *cache_file; upload_progress_cache_t *cache; } ServerConfig; @@ -480,7 +479,6 @@ apr_status_t upload_progress_cache_init(apr_pool_t *pool, ServerConfig *config) { /**/up_log(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_cache_init()"); -#if APR_HAS_SHARED_MEMORY apr_status_t result; apr_size_t size; upload_progress_cache_t *cache; @@ -531,7 +529,6 @@ apr_status_t upload_progress_cache_init(apr_pool_t *pool, ServerConfig *config) ap_log_error(APLOG_MARK, APLOG_INFO, 0, global_server, "Upload Progress: monitoring %i simultaneous uploads", nodes_cnt); -#endif return APR_SUCCESS; } @@ -559,7 +556,6 @@ int upload_progress_init(apr_pool_t *p, apr_pool_t *plog, apr_pool_userdata_set((const void *)1, userdata_key, apr_pool_cleanup_null, s->process->pool); - #if APR_HAS_SHARED_MEMORY /* If the cache file already exists then delete it. Otherwise we are * going to run into problems creating the shared memory. */ if (config->cache_file) { @@ -567,16 +563,13 @@ int upload_progress_init(apr_pool_t *p, apr_pool_t *plog, NULL); apr_file_remove(lck_file, ptemp); } - #endif return OK; } -#if APR_HAS_SHARED_MEMORY /* initializing cache if shared memory size is not zero and we already * don't have shm address */ if (!config->cache_shm && config->cache_bytes > 0) { -#endif result = upload_progress_cache_init(p, config); if (result != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, result, s, @@ -584,12 +577,10 @@ int upload_progress_init(apr_pool_t *p, apr_pool_t *plog, return DONE; } -#if APR_HAS_SHARED_MEMORY if (config->cache_file) { config->lock_file = apr_pstrcat(config->pool, config->cache_file, ".lck", NULL); } -#endif result = apr_global_mutex_create(&config->cache_lock, config->lock_file, APR_LOCK_DEFAULT, @@ -614,7 +605,6 @@ int upload_progress_init(apr_pool_t *p, apr_pool_t *plog, ap_get_module_config(s_vhost->module_config, &upload_progress_module); -#if APR_HAS_SHARED_MEMORY st_vhost->cache_shm = config->cache_shm; st_vhost->cache_rmm = config->cache_rmm; st_vhost->cache_file = config->cache_file; @@ -623,20 +613,17 @@ int upload_progress_init(apr_pool_t *p, apr_pool_t *plog, "Upload Progress: merging Shared Cache conf: shm=0x%pp rmm=0x%pp " "for VHOST: %s", config->cache_shm, config->cache_rmm, s_vhost->server_hostname); -#endif st_vhost->cache_lock = config->cache_lock; st_vhost->lock_file = config->lock_file; s_vhost = s_vhost->next; } -#if APR_HAS_SHARED_MEMORY } else { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "Upload Progress cache: cache size is zero, disabling " "shared memory cache"); } -#endif return(OK); } From fdfa8bf91d78bf76be309d39cfb697f8949b4bc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Mon, 24 Jan 2011 10:56:55 +0100 Subject: [PATCH 20/21] reportuploads_handler should always emit debug info MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index 5656f26..47382ae 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -649,12 +649,12 @@ static int reportuploads_handler(request_rec *r) const char *id = get_progress_id(r); if (id == NULL) { - up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, - "Upload Progress: Request without id in location with reports enabled. uri=%s", id, r->uri); + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, + "Upload Progress: Report requested without id. uri=%s", id, r->uri); return HTTP_NOT_FOUND; } else { - up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, - "Upload Progress: Request with id=%s in location with reports enabled. uri=%s", id, r->uri); + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, + "Upload Progress: Report requested with id=%s. uri=%s", id, r->uri); } ServerConfig *config = (ServerConfig*)ap_get_module_config(r->server->module_config, &upload_progress_module); From 307413e0231385a80906b1380002145311f25e55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pokrywka?= Date: Mon, 24 Jan 2011 12:14:28 +0100 Subject: [PATCH 21/21] Move common code extracting server config to separate function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Pokrywka --- mod_upload_progress.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/mod_upload_progress.c b/mod_upload_progress.c index 47382ae..210d2b4 100644 --- a/mod_upload_progress.c +++ b/mod_upload_progress.c @@ -161,8 +161,8 @@ static void upload_progress_register_hooks (apr_pool_t *p) ap_register_input_filter("UPLOAD_PROGRESS", track_upload_progress, NULL, AP_FTYPE_RESOURCE); } -ServerConfig *get_server_config(request_rec *r) { - return (ServerConfig*)ap_get_module_config(r->server->module_config, &upload_progress_module); +inline ServerConfig *get_server_config(server_rec *s) { + return (ServerConfig*)ap_get_module_config(s->module_config, &upload_progress_module); } static int upload_progress_handle_request(request_rec *r) @@ -170,7 +170,7 @@ static int upload_progress_handle_request(request_rec *r) /**/up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "upload_progress_handle_request()"); DirConfig* dir = (DirConfig*)ap_get_module_config(r->per_dir_config, &upload_progress_module); - ServerConfig *config = get_server_config(r); + ServerConfig *config = get_server_config(r->server); if (dir->track_enabled) { if (r->method_number == M_POST) { @@ -233,7 +233,7 @@ static const char *track_upload_progress_cmd(cmd_parms *cmd, void *config, int a static const char* upload_progress_shared_memory_size_cmd(cmd_parms *cmd, void *dummy, const char *arg) { /**/up_log(APLOG_MARK, APLOG_DEBUG, 0, global_server, "upload_progress_shared_memory_size_cmd()"); - ServerConfig *config = (ServerConfig*)ap_get_module_config(cmd->server->module_config, &upload_progress_module); + ServerConfig *config = get_server_config(cmd->server); long long int n = atoi(arg); @@ -287,7 +287,7 @@ static int track_upload_progress(ap_filter_t *f, apr_bucket_brigade *bb, /**/up_log(APLOG_MARK, APLOG_DEBUG, 0, global_server, "track_upload_progress()"); apr_status_t rv; upload_progress_node_t *node; - ServerConfig* config = get_server_config(f->r); + ServerConfig* config = get_server_config(f->r->server); rv = ap_get_brigade(f->next, bb, mode, block, readbytes); @@ -365,7 +365,7 @@ const char *get_json_callback_param(request_rec *r) { } } -int check_node(upload_progress_node_t *node, const char *key) { +inline int check_node(upload_progress_node_t *node, const char *key) { return strncasecmp(node->key, key, PROGRESS_KEY_LEN) == 0 ? 1 : 0; } @@ -388,7 +388,7 @@ void fill_new_upload_node_data(upload_progress_node_t *node, request_rec *r) { upload_progress_node_t* insert_node(request_rec *r, const char *key) { /**/up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "insert_node()"); - ServerConfig *config = (ServerConfig*)ap_get_module_config(r->server->module_config, &upload_progress_module); + ServerConfig *config = get_server_config(r->server); upload_progress_cache_t *cache = config->cache; upload_progress_node_t *node; @@ -408,7 +408,7 @@ upload_progress_node_t* insert_node(request_rec *r, const char *key) { upload_progress_node_t *find_node(request_rec *r, const char *key) { /**/up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "find_node()"); - ServerConfig *config = (ServerConfig*)ap_get_module_config(r->server->module_config, &upload_progress_module); + ServerConfig *config = get_server_config(r->server); upload_progress_cache_t *cache = config->cache; upload_progress_node_t *node, *nodes = cache->nodes; int *list = cache->list; @@ -442,7 +442,7 @@ static apr_status_t upload_progress_cleanup(void *data) static void clean_old_connections(request_rec *r) { /**/up_log(APLOG_MARK, APLOG_DEBUG, 0, r->server, "clean_old_connections()"); - ServerConfig *config = get_server_config(r); + ServerConfig *config = get_server_config(r->server); upload_progress_cache_t *cache = config->cache; upload_progress_node_t *node, *nodes = cache->nodes; int *list = cache->list; @@ -543,7 +543,7 @@ int upload_progress_init(apr_pool_t *p, apr_pool_t *plog, server_rec *s_vhost; ServerConfig *st_vhost; - ServerConfig *config = (ServerConfig*)ap_get_module_config(s->module_config, &upload_progress_module); + ServerConfig *config = get_server_config(s); void *data; const char *userdata_key = "upload_progress_init"; @@ -601,9 +601,7 @@ int upload_progress_init(apr_pool_t *p, apr_pool_t *plog, /* merge config in all vhost */ s_vhost = s->next; while (s_vhost) { - st_vhost = (ServerConfig *) - ap_get_module_config(s_vhost->module_config, - &upload_progress_module); + st_vhost = get_server_config(s_vhost); st_vhost->cache_shm = config->cache_shm; st_vhost->cache_rmm = config->cache_rmm; @@ -657,7 +655,7 @@ static int reportuploads_handler(request_rec *r) "Upload Progress: Report requested with id=%s. uri=%s", id, r->uri); } - ServerConfig *config = (ServerConfig*)ap_get_module_config(r->server->module_config, &upload_progress_module); + ServerConfig *config = get_server_config(r->server); CACHE_LOCK(); upload_progress_node_t *node = find_node(r, id); @@ -727,7 +725,7 @@ static void upload_progress_child_init(apr_pool_t *p, server_rec *s) /**/up_log(APLOG_MARK, APLOG_DEBUG, 0, s, "upload_progress_child_init()"); apr_status_t sts; - ServerConfig *st = (ServerConfig *)ap_get_module_config(s->module_config, &upload_progress_module); + ServerConfig *st = get_server_config(s); if (!st->cache_lock) { ap_log_error(APLOG_MARK, APLOG_CRIT, 0, s, "Global mutex not set.");