Author: mdounin
Date: 2012-02-27 22:15:39 +0000 (Mon, 27 Feb 2012)
New Revision: 4498

Log:
Fix of rbtree lookup on hash collisions.

Previous code incorrectly assumed that nodes with identical keys are linked
together. This might not be true after tree rebalance.

Patch by Lanshun Zhou.


Modified:
trunk/src/core/ngx_open_file_cache.c
trunk/src/core/ngx_resolver.c
trunk/src/event/ngx_event_openssl.c
trunk/src/http/modules/ngx_http_limit_conn_module.c
trunk/src/http/modules/ngx_http_limit_req_module.c
trunk/src/http/ngx_http_file_cache.c

Modified: trunk/src/core/ngx_open_file_cache.c
===================================================================
--- trunk/src/core/ngx_open_file_cache.c 2012-02-27 16:57:02 UTC (rev 4497)
+++ trunk/src/core/ngx_open_file_cache.c 2012-02-27 22:15:39 UTC (rev 4498)
@@ -1142,20 +1142,15 @@

/* hash == node->key */

- do {
- file = (ngx_cached_open_file_t *) node;
+ file = (ngx_cached_open_file_t *) node;

- rc = ngx_strcmp(name->data, file->name);
+ rc = ngx_strcmp(name->data, file->name);

- if (rc == 0) {
- return file;
- }
+ if (rc == 0) {
+ return file;
+ }

- node = (rc < 0) ? node->left : node->right;
-
- } while (node != sentinel && hash == node->key);
-
- break;
+ node = (rc < 0) ? node->left : node->right;
}

return NULL;

Modified: trunk/src/core/ngx_resolver.c
===================================================================
--- trunk/src/core/ngx_resolver.c 2012-02-27 16:57:02 UTC (rev 4497)
+++ trunk/src/core/ngx_resolver.c 2012-02-27 22:15:39 UTC (rev 4498)
@@ -1689,20 +1689,15 @@

/* hash == node->key */

- do {
- rn = (ngx_resolver_node_t *) node;
+ rn = (ngx_resolver_node_t *) node;

- rc = ngx_memn2cmp(name->data, rn->name, name->len, rn->nlen);
+ rc = ngx_memn2cmp(name->data, rn->name, name->len, rn->nlen);

- if (rc == 0) {
- return rn;
- }
+ if (rc == 0) {
+ return rn;
+ }

- node = (rc < 0) ? node->left : node->right;
-
- } while (node != sentinel && hash == node->key);
-
- break;
+ node = (rc < 0) ? node->left : node->right;
}

/* not found */

Modified: trunk/src/event/ngx_event_openssl.c
===================================================================
--- trunk/src/event/ngx_event_openssl.c 2012-02-27 16:57:02 UTC (rev 4497)
+++ trunk/src/event/ngx_event_openssl.c 2012-02-27 22:15:39 UTC (rev 4498)
@@ -1801,44 +1801,39 @@

/* hash == node->key */

- do {
- sess_id = (ngx_ssl_sess_id_t *) node;
+ sess_id = (ngx_ssl_sess_id_t *) node;

- rc = ngx_memn2cmp(id, sess_id->id,
- (size_t) len, (size_t) node->data);
- if (rc == 0) {
+ rc = ngx_memn2cmp(id, sess_id->id, (size_t) len, (size_t) node->data);

- if (sess_id->expire > ngx_time()) {
- ngx_memcpy(buf, sess_id->session, sess_id->len);
+ if (rc == 0) {

- ngx_shmtx_unlock(&shpool->mutex);
+ if (sess_id->expire > ngx_time()) {
+ ngx_memcpy(buf, sess_id->session, sess_id->len);

- p = buf;
- sess = d2i_SSL_SESSION(NULL, &p, sess_id->len);
+ ngx_shmtx_unlock(&shpool->mutex);

- return sess;
- }
+ p = buf;
+ sess = d2i_SSL_SESSION(NULL, &p, sess_id->len);

- ngx_queue_remove(&sess_id->queue);
+ return sess;
+ }

- ngx_rbtree_delete(&cache->session_rbtree, node);
+ ngx_queue_remove(&sess_id->queue);

- ngx_slab_free_locked(shpool, sess_id->session);
+ ngx_rbtree_delete(&cache->session_rbtree, node);
+
+ ngx_slab_free_locked(shpool, sess_id->session);
#if (NGX_PTR_SIZE == 4)
- ngx_slab_free_locked(shpool, sess_id->id);
+ ngx_slab_free_locked(shpool, sess_id->id);
#endif
- ngx_slab_free_locked(shpool, sess_id);
+ ngx_slab_free_locked(shpool, sess_id);

- sess = NULL;
+ sess = NULL;

- goto done;
- }
+ goto done;
+ }

- node = (rc < 0) ? node->left : node->right;
-
- } while (node != sentinel && hash == node->key);
-
- break;
+ node = (rc < 0) ? node->left : node->right;
}

done:
@@ -1908,31 +1903,26 @@

/* hash == node->key */

- do {
- sess_id = (ngx_ssl_sess_id_t *) node;
+ sess_id = (ngx_ssl_sess_id_t *) node;

- rc = ngx_memn2cmp(id, sess_id->id, len, (size_t) node->data);
+ rc = ngx_memn2cmp(id, sess_id->id, len, (size_t) node->data);

- if (rc == 0) {
+ if (rc == 0) {

- ngx_queue_remove(&sess_id->queue);
+ ngx_queue_remove(&sess_id->queue);

- ngx_rbtree_delete(&cache->session_rbtree, node);
+ ngx_rbtree_delete(&cache->session_rbtree, node);

- ngx_slab_free_locked(shpool, sess_id->session);
+ ngx_slab_free_locked(shpool, sess_id->session);
#if (NGX_PTR_SIZE == 4)
- ngx_slab_free_locked(shpool, sess_id->id);
+ ngx_slab_free_locked(shpool, sess_id->id);
#endif
- ngx_slab_free_locked(shpool, sess_id);
+ ngx_slab_free_locked(shpool, sess_id);

- goto done;
- }
+ goto done;
+ }

- node = (rc < 0) ? node->left : node->right;
-
- } while (node != sentinel && hash == node->key);
-
- break;
+ node = (rc < 0) ? node->left : node->right;
}

done:

Modified: trunk/src/http/modules/ngx_http_limit_conn_module.c
===================================================================
--- trunk/src/http/modules/ngx_http_limit_conn_module.c 2012-02-27 16:57:02 UTC (rev 4497)
+++ trunk/src/http/modules/ngx_http_limit_conn_module.c 2012-02-27 22:15:39 UTC (rev 4498)
@@ -325,20 +325,15 @@

/* hash == node->key */

- do {
- lcn = (ngx_http_limit_conn_node_t *) &node->color;
+ lcn = (ngx_http_limit_conn_node_t *) &node->color;

- rc = ngx_memn2cmp(vv->data, lcn->data,
- (size_t) vv->len, (size_t) lcn->len);
- if (rc == 0) {
- return node;
- }
+ rc = ngx_memn2cmp(vv->data, lcn->data,
+ (size_t) vv->len, (size_t) lcn->len);
+ if (rc == 0) {
+ return node;
+ }

- node = (rc < 0) ? node->left : node->right;
-
- } while (node != sentinel && hash == node->key);
-
- break;
+ node = (rc < 0) ? node->left : node->right;
}

return NULL;

Modified: trunk/src/http/modules/ngx_http_limit_req_module.c
===================================================================
--- trunk/src/http/modules/ngx_http_limit_req_module.c 2012-02-27 16:57:02 UTC (rev 4497)
+++ trunk/src/http/modules/ngx_http_limit_req_module.c 2012-02-27 22:15:39 UTC (rev 4498)
@@ -385,47 +385,42 @@

/* hash == node->key */

- do {
- lr = (ngx_http_limit_req_node_t *) &node->color;
+ lr = (ngx_http_limit_req_node_t *) &node->color;

- rc = ngx_memn2cmp(data, lr->data, len, (size_t) lr->len);
+ rc = ngx_memn2cmp(data, lr->data, len, (size_t) lr->len);

- if (rc == 0) {
- ngx_queue_remove(&lr->queue);
- ngx_queue_insert_head(&ctx->sh->queue, &lr->queue);
+ if (rc == 0) {
+ ngx_queue_remove(&lr->queue);
+ ngx_queue_insert_head(&ctx->sh->queue, &lr->queue);

- ms = (ngx_msec_int_t) (now - lr->last);
+ ms = (ngx_msec_int_t) (now - lr->last);

- excess = lr->excess - ctx->rate * ngx_abs(ms) / 1000 + 1000;
+ excess = lr->excess - ctx->rate * ngx_abs(ms) / 1000 + 1000;

- if (excess < 0) {
- excess = 0;
- }
+ if (excess < 0) {
+ excess = 0;
+ }

- *ep = excess;
+ *ep = excess;

- if ((ngx_uint_t) excess > limit->burst) {
- return NGX_BUSY;
- }
+ if ((ngx_uint_t) excess > limit->burst) {
+ return NGX_BUSY;
+ }

- if (account) {
- lr->excess = excess;
- lr->last = now;
- return NGX_OK;
- }
-
- lr->count++;
-
- ctx->node = lr;
-
- return NGX_AGAIN;
+ if (account) {
+ lr->excess = excess;
+ lr->last = now;
+ return NGX_OK;
}

- node = (rc < 0) ? node->left : node->right;
+ lr->count++;

- } while (node != sentinel && hash == node->key);
+ ctx->node = lr;

- break;
+ return NGX_AGAIN;
+ }
+
+ node = (rc < 0) ? node->left : node->right;
}

*ep = 0;

Modified: trunk/src/http/ngx_http_file_cache.c
===================================================================
--- trunk/src/http/ngx_http_file_cache.c 2012-02-27 16:57:02 UTC (rev 4497)
+++ trunk/src/http/ngx_http_file_cache.c 2012-02-27 22:15:39 UTC (rev 4498)
@@ -799,21 +799,16 @@

/* node_key == node->key */

- do {
- fcn = (ngx_http_file_cache_node_t *) node;
+ fcn = (ngx_http_file_cache_node_t *) node;

- rc = ngx_memcmp(&key[sizeof(ngx_rbtree_key_t)], fcn->key,
- NGX_HTTP_CACHE_KEY_LEN - sizeof(ngx_rbtree_key_t));
+ rc = ngx_memcmp(&key[sizeof(ngx_rbtree_key_t)], fcn->key,
+ NGX_HTTP_CACHE_KEY_LEN - sizeof(ngx_rbtree_key_t));

- if (rc == 0) {
- return fcn;
- }
+ if (rc == 0) {
+ return fcn;
+ }

- node = (rc < 0) ? node->left : node->right;
-
- } while (node != sentinel && node_key == node->key);
-
- break;
+ node = (rc < 0) ? node->left : node->right;
}

/* not found */

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel