From: horus <horus.li@gmail.com>
Date: Tue, 28 Jul 2026 02:46:59 +0000 (+0800)
Subject: Modules: fix SharedDict.pop() for unexpired keys in timeout zones
X-Git-Tag: 1.0.1~21
X-Git-Url: https://git.kaiwu.me/%7B@url%7D?a=commitdiff_plain;h=4f4eca5e8e522b27f3edb5bde1d084c6c23831d4;p=njs.git

Modules: fix SharedDict.pop() for unexpired keys in timeout zones

Previously, SharedDict.pop() removed an entry from the expiration rbtree
before checking whether it had expired. ngx_rbtree_delete() clears the
removed node's key, so the subsequent expiration check treated every
entry in a timeout zone as expired. pop() therefore deleted unexpired
entries but returned undefined instead of their values.

The fix preserves the expiration key before removing the node and uses
the saved value for the check in both the njs and QuickJS
implementations. Tests are added for popping an unexpired entry from a
timeout zone.
---

diff --git a/nginx/ngx_js_shared_dict.c b/nginx/ngx_js_shared_dict.c
index d9a1fc7c..b4b16b98 100644
--- a/nginx/ngx_js_shared_dict.c
+++ b/nginx/ngx_js_shared_dict.c
@@ -1429,6 +1429,20 @@ ngx_js_dict_node_free(ngx_js_dict_t *dict, ngx_js_dict_node_t *node)
 }
 
 
+static ngx_rbtree_key_t
+ngx_js_dict_expire_unlink(ngx_js_dict_t *dict, ngx_js_dict_node_t *node)
+{
+    ngx_rbtree_key_t  expire;
+
+    /* ngx_rbtree_delete() clears the node key. */
+    expire = node->expire.key;
+
+    ngx_rbtree_delete(&dict->sh->rbtree_expire, &node->expire);
+
+    return expire;
+}
+
+
 static ngx_int_t
 ngx_js_dict_set(njs_vm_t *vm, ngx_js_dict_t *dict, ngx_str_t *key,
     njs_value_t *value, ngx_msec_t timeout, unsigned flags)
@@ -1618,6 +1632,7 @@ ngx_js_dict_delete(njs_vm_t *vm, ngx_js_dict_t *dict, ngx_str_t *key,
     ngx_int_t            rc;
     ngx_msec_t           now;
     ngx_time_t          *tp;
+    ngx_rbtree_key_t     expire;
     ngx_js_dict_node_t  *node;
 
     ngx_rwlock_wlock(&dict->sh->rwlock);
@@ -1629,8 +1644,10 @@ ngx_js_dict_delete(njs_vm_t *vm, ngx_js_dict_t *dict, ngx_str_t *key,
         return NGX_DECLINED;
     }
 
+    expire = 0;
+
     if (dict->timeout) {
-        ngx_rbtree_delete(&dict->sh->rbtree_expire, &node->expire);
+        expire = ngx_js_dict_expire_unlink(dict, node);
     }
 
     ngx_rbtree_delete(&dict->sh->rbtree, (ngx_rbtree_node_t *) node);
@@ -1639,7 +1656,7 @@ ngx_js_dict_delete(njs_vm_t *vm, ngx_js_dict_t *dict, ngx_str_t *key,
         tp = ngx_timeofday();
         now = tp->sec * 1000 + tp->msec;
 
-        if (!dict->timeout || now < node->expire.key) {
+        if (!dict->timeout || now < expire) {
             rc = ngx_js_dict_copy_value_locked(vm, dict, node, retval);
 
         } else {
@@ -4041,6 +4058,7 @@ ngx_qjs_dict_delete(JSContext *cx, ngx_js_dict_t *dict, ngx_str_t *key,
     JSValue              ret;
     ngx_msec_t           now;
     ngx_time_t          *tp;
+    ngx_rbtree_key_t     expire;
     ngx_js_dict_node_t  *node;
 
     ngx_rwlock_wlock(&dict->sh->rwlock);
@@ -4052,8 +4070,10 @@ ngx_qjs_dict_delete(JSContext *cx, ngx_js_dict_t *dict, ngx_str_t *key,
         return JS_UNDEFINED;
     }
 
+    expire = 0;
+
     if (dict->timeout) {
-        ngx_rbtree_delete(&dict->sh->rbtree_expire, &node->expire);
+        expire = ngx_js_dict_expire_unlink(dict, node);
     }
 
     ngx_rbtree_delete(&dict->sh->rbtree, (ngx_rbtree_node_t *) node);
@@ -4062,7 +4082,7 @@ ngx_qjs_dict_delete(JSContext *cx, ngx_js_dict_t *dict, ngx_str_t *key,
         tp = ngx_timeofday();
         now = tp->sec * 1000 + tp->msec;
 
-        if (!dict->timeout || now < node->expire.key) {
+        if (!dict->timeout || now < expire) {
             ret = ngx_qjs_dict_copy_value_locked(cx, dict, node);
 
         } else {
diff --git a/nginx/t/js_shared_dict.t b/nginx/t/js_shared_dict.t
index b30e2865..0fb05db4 100644
--- a/nginx/t/js_shared_dict.t
+++ b/nginx/t/js_shared_dict.t
@@ -356,7 +356,7 @@ EOF
 
 $t->try_run('no js_shared_dict_zone');
 
-$t->plan(59);
+$t->plan(61);
 
 ###############################################################################
 
@@ -482,6 +482,11 @@ ok($ttl_incr_def >= 900000 && $ttl_incr_def <= 1000000,
 
 like(http_get('/pop?dict=bar&key=FOO'), qr/zzz/, 'pop bar.FOO');
 like(http_get('/pop?dict=bar&key=FOO'), qr/undefined/, 'pop deleted bar.FOO');
+
+http_get('/set?dict=foo&key=POP&value=alive');
+like(http_get('/pop?dict=foo&key=POP'), qr/alive/, 'pop unexpired foo.POP');
+like(http_get('/pop?dict=foo&key=POP'), qr/undefined/, 'pop deleted foo.POP');
+
 http_get('/set?dict=foo&key=BAR&value=xxx');
 like(http_get('/clear?dict=foo'), qr/undefined/, 'clear foo');