From ea47086628fa013efd109394b46bb17c4ce4f49b Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Fri, 19 Nov 2010 20:24:20 +0100 Subject: [PATCH 1/5] This is a temp commit to log the rb tree to chase issue #7 root cause --- ngx_http_uploadprogress_module.c | 43 ++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/ngx_http_uploadprogress_module.c b/ngx_http_uploadprogress_module.c index 0dba197..64353e1 100644 --- a/ngx_http_uploadprogress_module.c +++ b/ngx_http_uploadprogress_module.c @@ -347,6 +347,40 @@ get_tracking_id(ngx_http_request_t * r) return NULL; } + +static void log_node(ngx_rbtree_node_t *node, ngx_log_t * log) +{ + ngx_http_uploadprogress_node_t *up; + up = (ngx_http_uploadprogress_node_t *) node; + + ngx_log_debug4(NGX_LOG_DEBUG_HTTP, log, 0, "upload-progress: find_node %s [%uO/%uO,%d,%d]",up->data, up->rest, up->length, up->err_status, up->timeout ); +} + +static void log_rbtree(ngx_http_uploadprogress_ctx_t * ctx, ngx_log_t * log) +{ + ngx_http_uploadprogress_node_t *up; + ngx_rbtree_node_t *node; + + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, log, 0, + "upload-progress: logging the whole rbtree"); + + node = (ngx_rbtree_node_t *) ctx->list_tail.prev; + for (;;) { + if (node == &ctx->list_head.node) { + break; + } + + up = (ngx_http_uploadprogress_node_t *) node; + + log_node(node, log); + + node = (ngx_rbtree_node_t *)up->prev; + } + + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, log, 0, + "upload-progress: end logging the whole rbtree"); +} + static ngx_http_uploadprogress_node_t * find_node(ngx_str_t * id, ngx_http_uploadprogress_ctx_t * ctx, ngx_log_t * log) { @@ -358,23 +392,30 @@ find_node(ngx_str_t * id, ngx_http_uploadprogress_ctx_t * ctx, ngx_log_t * log) ngx_log_debug1(NGX_LOG_DEBUG_HTTP, log, 0, "upload-progress: find_node %V", id); hash = ngx_crc32_short(id->data, id->len); + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, log, 0, "upload-progress: find_node hash %08XD", hash); node = ctx->rbtree->root; sentinel = ctx->rbtree->sentinel; while (node != sentinel) { + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, log, 0, "upload-progress: find_node testing hash %08XD", node->key); + log_node(node, log); + if (hash < node->key) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, log, 0, "upload-progress: find_node going left"); node = node->left; continue; } if (hash > node->key) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, log, 0, "upload-progress: find_node going right"); node = node->right; continue; } /* hash == node->key */ + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, log, 0, "upload-progress: find_node hash matching"); do { up = (ngx_http_uploadprogress_node_t *) node; @@ -392,6 +433,7 @@ find_node(ngx_str_t * id, ngx_http_uploadprogress_ctx_t * ctx, ngx_log_t * log) } while (node != sentinel && hash == node->key); /* found a key with unmatching hash (and value), let's keep comparing hashes then */ + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, log, 0, "upload-progress: find_node hash not matching anymore"); } ngx_log_debug0(NGX_LOG_DEBUG_HTTP, log, 0, "upload-progress: can't find node"); return NULL; @@ -594,6 +636,7 @@ ngx_http_reportuploads_handler(ngx_http_request_t * r) } else { ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "reportuploads not found: %V", id); + log_rbtree(ctx,r->connection->log); } ngx_shmtx_unlock(&shpool->mutex); ngx_free(id); From 83250cb92b0a1edafc8b7dd68c31b2b63ee9ece9 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Thu, 25 Nov 2010 20:41:17 +0100 Subject: [PATCH 2/5] More debug to find the root cause of issue #7 --- ngx_http_uploadprogress_module.c | 47 ++++++++++++-------------------- 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/ngx_http_uploadprogress_module.c b/ngx_http_uploadprogress_module.c index 64353e1..cb59860 100644 --- a/ngx_http_uploadprogress_module.c +++ b/ngx_http_uploadprogress_module.c @@ -917,54 +917,36 @@ ngx_http_uploadprogress_rbtree_insert_value(ngx_rbtree_node_t * temp, ngx_rbtree_node_t * node, ngx_rbtree_node_t * sentinel) { + ngx_rbtree_node_t **p; ngx_http_uploadprogress_node_t *upn, *upnt; for (;;) { if (node->key < temp->key) { - if (temp->left == sentinel) { - temp->left = node; - break; - } - - temp = temp->left; + p = &temp->left; } else if (node->key > temp->key) { - if (temp->right == sentinel) { - temp->right = node; - break; - } - - temp = temp->right; + p = &temp->right; - } else { /* node->key == temp->key */ + } else { /* node->key == temp->key */ upn = (ngx_http_uploadprogress_node_t *) node; upnt = (ngx_http_uploadprogress_node_t *) temp; - if (ngx_memn2cmp(upn->data, upnt->data, upn->len, upnt->len) < 0) { - - if (temp->left == sentinel) { - temp->left = node; - break; - } - - temp = temp->left; - - } else { - - if (temp->right == sentinel) { - temp->right = node; - break; - } + p = (ngx_memn2cmp(upn->data, upnt->data, upn->len, upnt->len) < 0) + ? &temp->left : &temp->right; + } - temp = temp->right; - } + if (*p == sentinel) { + break; } + + temp = *p; } + *p = node; node->parent = temp; node->left = sentinel; node->right = sentinel; @@ -1216,6 +1198,9 @@ ngx_http_uploadprogress_errortracker(ngx_http_request_t * r) ctx->list_head.next = up; ngx_rbtree_insert(ctx->rbtree, node); + + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "trackuploads error-tracking: %08XD inserted in rbtree", hash); /* start the timer if needed */ if (!upcf->cleanup.timer_set) { @@ -1225,6 +1210,8 @@ ngx_http_uploadprogress_errortracker(ngx_http_request_t * r) ngx_add_timer(&upcf->cleanup, TIMER_FREQUENCY); } + log_rbtree(ctx,r->connection->log); + ngx_shmtx_unlock(&shpool->mutex); cln->handler = ngx_http_uploadprogress_cleanup; From cd8edb6ef9a6f01404cc7a1bffa865b8e3809012 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Fri, 26 Nov 2010 12:35:33 +0100 Subject: [PATCH 3/5] attempt to fix failure in insert --- ngx_http_uploadprogress_module.c | 50 ++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/ngx_http_uploadprogress_module.c b/ngx_http_uploadprogress_module.c index cb59860..52c724f 100644 --- a/ngx_http_uploadprogress_module.c +++ b/ngx_http_uploadprogress_module.c @@ -917,36 +917,54 @@ ngx_http_uploadprogress_rbtree_insert_value(ngx_rbtree_node_t * temp, ngx_rbtree_node_t * node, ngx_rbtree_node_t * sentinel) { - ngx_rbtree_node_t **p; - ngx_http_uploadprogress_node_t *upn, *upnt; + ngx_http_uploadprogress_node_t *upn, *upnt; - for (;;) { + for (;;) { + + if (node->key < temp->key) { - if (node->key < temp->key) { + if (temp->left == sentinel) { + temp->left = node; + break; + } - p = &temp->left; + temp = temp->left; } else if (node->key > temp->key) { - p = &temp->right; + if (temp->right == sentinel) { + temp->right = node; + break; + } - } else { /* node->key == temp->key */ + temp = temp->right; + + } else { /* node->key == temp->key */ upn = (ngx_http_uploadprogress_node_t *) node; upnt = (ngx_http_uploadprogress_node_t *) temp; - p = (ngx_memn2cmp(upn->data, upnt->data, upn->len, upnt->len) < 0) - ? &temp->left : &temp->right; - } + if (ngx_memn2cmp(upn->data, upnt->data, upn->len, upnt->len) < 0) { - if (*p == sentinel) { - break; - } + if (temp->left == sentinel) { + temp->left = node; + break; + } - temp = *p; - } + temp = temp->left; + + } else { + + if (temp->right == sentinel) { + temp->right = node; + break; + } - *p = node; + temp = temp->right; + } + } + } + node->parent = temp; node->left = sentinel; node->right = sentinel; From ba44ec797935e5fd8954cdc8848df61f111eb63a Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Fri, 26 Nov 2010 14:40:41 +0100 Subject: [PATCH 4/5] rbtree insertion debug log --- ngx_http_uploadprogress_module.c | 69 ++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/ngx_http_uploadprogress_module.c b/ngx_http_uploadprogress_module.c index 52c724f..3152280 100644 --- a/ngx_http_uploadprogress_module.c +++ b/ngx_http_uploadprogress_module.c @@ -34,6 +34,7 @@ struct ngx_http_uploadprogress_node_s { time_t timeout; struct ngx_http_uploadprogress_node_s *prev; struct ngx_http_uploadprogress_node_s *next; + ngx_log_t *log; u_char len; u_char data[1]; }; @@ -870,6 +871,7 @@ ngx_http_uploadprogress_handler(ngx_http_request_t * r) up->rest = 0; up->length = 0; up->timeout = 0; + up->log = NULL; up->next = ctx->list_head.next; up->next->prev = up; @@ -918,12 +920,35 @@ ngx_http_uploadprogress_rbtree_insert_value(ngx_rbtree_node_t * temp, ngx_rbtree_node_t * sentinel) { ngx_http_uploadprogress_node_t *upn, *upnt; + ngx_log_t *log = NULL; + + log = ((ngx_http_uploadprogress_node_t *)node)->log; + + if (log != NULL) { + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, log, 0, + "upload-progree: rbtree insert %08XD to be inserted", node->key); + log_node(node, log); + } for (;;) { + if (log != NULL) { + ngx_log_debug2(NGX_LOG_DEBUG_HTTP, log, 0, + "upload-progree: rbtree insert %08XD compared to %08XD", node->key, temp->key); + log_node(temp, log); + } if (node->key < temp->key) { + if (log != NULL) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, log, 0, + "upload-progree: rbtree insert going left"); + } + if (temp->left == sentinel) { + if (log != NULL) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, log, 0, + "upload-progree: rbtree insert left end of tree"); + } temp->left = node; break; } @@ -932,7 +957,16 @@ ngx_http_uploadprogress_rbtree_insert_value(ngx_rbtree_node_t * temp, } else if (node->key > temp->key) { + if (log != NULL) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, log, 0, + "upload-progree: rbtree insert going right"); + } + if (temp->right == sentinel) { + if (log != NULL) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, log, 0, + "upload-progree: rbtree insert right end of tree"); + } temp->right = node; break; } @@ -941,12 +975,25 @@ ngx_http_uploadprogress_rbtree_insert_value(ngx_rbtree_node_t * temp, } else { /* node->key == temp->key */ + if (log != NULL) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, log, 0, + "upload-progree: equal hashes"); + } upn = (ngx_http_uploadprogress_node_t *) node; upnt = (ngx_http_uploadprogress_node_t *) temp; if (ngx_memn2cmp(upn->data, upnt->data, upn->len, upnt->len) < 0) { + if (log != NULL) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, log, 0, + "upload-progree: rbtree insert eq h node < temp, going left"); + } + if (temp->left == sentinel) { + if (log != NULL) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, log, 0, + "upload-progree: rbtree insert end of left"); + } temp->left = node; break; } @@ -955,7 +1002,15 @@ ngx_http_uploadprogress_rbtree_insert_value(ngx_rbtree_node_t * temp, } else { + if (log != NULL) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, log, 0, + "upload-progree: rbtree insert eq h node > temp, going right"); + } if (temp->right == sentinel) { + if (log != NULL) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, log, 0, + "upload-progree: rbtree insert end of right"); + } temp->right = node; break; } @@ -964,6 +1019,11 @@ ngx_http_uploadprogress_rbtree_insert_value(ngx_rbtree_node_t * temp, } } } + + if (log != NULL) { + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, log, 0, + "upload-progree: rbtree insert insert below %08XD", temp->key); + } node->parent = temp; node->left = sentinel; @@ -1208,6 +1268,7 @@ ngx_http_uploadprogress_errortracker(ngx_http_request_t * r) node->key = hash; up->len = (u_char) id->len; up->err_status = r->err_status; + up->log = r->connection->log; ngx_memcpy(up->data, id->data, id->len); up->next = ctx->list_head.next; @@ -1220,6 +1281,14 @@ ngx_http_uploadprogress_errortracker(ngx_http_request_t * r) ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "trackuploads error-tracking: %08XD inserted in rbtree", hash); + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "trackuploads error-tracking: checking if %08XD is correctly inserted in rbtree", hash); + + if (find_node(id, ctx, r->connection->log) == NULL) { + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "trackuploads error-tracking: %08XD hasn't been correctly added, this suggests an rbtree issue", hash); + } + /* start the timer if needed */ if (!upcf->cleanup.timer_set) { upcf->cleanup.data = upcf->shm_zone; From 680dbd742d8ac4d553d1e8b1cc46669f9635daf9 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Fri, 26 Nov 2010 15:16:06 +0100 Subject: [PATCH 5/5] Attempt to fix issue #7 --- ngx_http_uploadprogress_module.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ngx_http_uploadprogress_module.c b/ngx_http_uploadprogress_module.c index 3152280..bd44da9 100644 --- a/ngx_http_uploadprogress_module.c +++ b/ngx_http_uploadprogress_module.c @@ -1268,6 +1268,10 @@ ngx_http_uploadprogress_errortracker(ngx_http_request_t * r) node->key = hash; up->len = (u_char) id->len; up->err_status = r->err_status; + up->done = 0; + up->rest = 0; + up->length = 0; + up->timeout = 0; up->log = r->connection->log; ngx_memcpy(up->data, id->data, id->len);