Modules: removed shared dict expiration from read-locked paths.

Previously, keys(), items(), and size() called ngx_js_dict_expire()
under a read lock.  Since ngx_js_dict_expire() deletes nodes from
both rbtrees and frees slab memory, concurrent readers on different
worker processes could corrupt shared memory by freeing the same
expired nodes simultaneously.

The fix removes ngx_js_dict_expire() calls from all read-locked
paths and instead skips expired entries during iteration, consistent
with how get() and has() already handle expiry.  Actual cleanup of
expired entries is deferred to write-side operations (set, add,
delete, clear).

diff --git a/nginx/ngx_js_shared_dict.c b/nginx/ngx_js_shared_dict.c
index 54af5028d00ab07c673a8d00b0b3183afbba0728..c57ac90385d2d1bed006acb586d7054881c9b68a 100644 (file)
--- a/nginx/ngx_js_shared_dict.c
+++ b/nginx/ngx_js_shared_dict.c
@@ -841,14 +841,15 @@ njs_js_ext_shared_dict_keys(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
         return NJS_ERROR;
     }
 
-    ngx_rwlock_rlock(&dict->sh->rwlock);
+    now = 0;
 
     if (dict->timeout) {
         tp = ngx_timeofday();
         now = tp->sec * 1000 + tp->msec;
-        ngx_js_dict_expire(dict, now);
     }
 
+    ngx_rwlock_rlock(&dict->sh->rwlock);
+
     rbtree = &dict->sh->rbtree;
 
     if (rbtree->root == rbtree->sentinel) {
@@ -859,12 +860,16 @@ njs_js_ext_shared_dict_keys(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
          rn != NULL;
          rn = ngx_rbtree_next(rbtree, rn))
     {
+        node = (ngx_js_dict_node_t *) rn;
+
+        if (dict->timeout && now >= node->expire.key) {
+            continue;
+        }
+
         if (max_count-- == 0) {
             break;
         }
 
-        node = (ngx_js_dict_node_t *) rn;
-
         value = njs_vm_array_push(vm, retval);
         if (value == NULL) {
             goto fail;
@@ -1011,14 +1016,15 @@ njs_js_ext_shared_dict_items(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
         return NJS_ERROR;
     }
 
-    ngx_rwlock_rlock(&dict->sh->rwlock);
+    now = 0;
 
     if (dict->timeout) {
         tp = ngx_timeofday();
         now = tp->sec * 1000 + tp->msec;
-        ngx_js_dict_expire(dict, now);
     }
 
+    ngx_rwlock_rlock(&dict->sh->rwlock);
+
     rbtree = &dict->sh->rbtree;
 
     if (rbtree->root == rbtree->sentinel) {
@@ -1029,12 +1035,16 @@ njs_js_ext_shared_dict_items(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
          rn != NULL;
          rn = ngx_rbtree_next(rbtree, rn))
     {
+        node = (ngx_js_dict_node_t *) rn;
+
+        if (dict->timeout && now >= node->expire.key) {
+            continue;
+        }
+
         if (max_count-- == 0) {
             break;
         }
 
-        node = (ngx_js_dict_node_t *) rn;
-
         kv = njs_vm_array_push(vm, retval);
         if (kv == NULL) {
             goto fail;
@@ -1210,13 +1220,14 @@ static njs_int_t
 njs_js_ext_shared_dict_size(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
     njs_index_t unused, njs_value_t *retval)
 {
-    njs_int_t           items;
-    ngx_msec_t          now;
-    ngx_time_t         *tp;
-    ngx_rbtree_t       *rbtree;
-    ngx_js_dict_t      *dict;
-    ngx_shm_zone_t     *shm_zone;
-    ngx_rbtree_node_t  *rn;
+    njs_int_t            items;
+    ngx_msec_t           now;
+    ngx_time_t          *tp;
+    ngx_rbtree_t        *rbtree;
+    ngx_js_dict_t       *dict;
+    ngx_shm_zone_t      *shm_zone;
+    ngx_rbtree_node_t   *rn;
+    ngx_js_dict_node_t  *node;
 
     shm_zone = njs_vm_external(vm, ngx_js_shared_dict_proto_id,
                                njs_argument(args, 0));
@@ -1227,14 +1238,15 @@ njs_js_ext_shared_dict_size(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
 
     dict = shm_zone->data;
 
-    ngx_rwlock_rlock(&dict->sh->rwlock);
+    now = 0;
 
     if (dict->timeout) {
         tp = ngx_timeofday();
         now = tp->sec * 1000 + tp->msec;
-        ngx_js_dict_expire(dict, now);
     }
 
+    ngx_rwlock_rlock(&dict->sh->rwlock);
+
     rbtree = &dict->sh->rbtree;
 
     if (rbtree->root == rbtree->sentinel) {
@@ -1249,6 +1261,12 @@ njs_js_ext_shared_dict_size(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
          rn != NULL;
          rn = ngx_rbtree_next(rbtree, rn))
     {
+        node = (ngx_js_dict_node_t *) rn;
+
+        if (dict->timeout && now >= node->expire.key) {
+            continue;
+        }
+
         items++;
     }
 
@@ -3528,14 +3546,15 @@ ngx_qjs_ext_shared_dict_items(JSContext *cx, JSValueConst this_val,
 
     rbtree = &dict->sh->rbtree;
 
-    ngx_rwlock_rlock(&dict->sh->rwlock);
+    now = 0;
 
     if (dict->timeout) {
         tp = ngx_timeofday();
         now = tp->sec * 1000 + tp->msec;
-        ngx_js_dict_expire(dict, now);
     }
 
+    ngx_rwlock_rlock(&dict->sh->rwlock);
+
     if (rbtree->root == rbtree->sentinel) {
         ngx_rwlock_unlock(&dict->sh->rwlock);
         return JS_NewArray(cx);
@@ -3553,12 +3572,16 @@ ngx_qjs_ext_shared_dict_items(JSContext *cx, JSValueConst this_val,
          rn != NULL;
          rn = ngx_rbtree_next(rbtree, rn))
     {
+        node = (ngx_js_dict_node_t *) rn;
+
+        if (dict->timeout && now >= node->expire.key) {
+            continue;
+        }
+
         if (max_count-- == 0) {
             break;
         }
 
-        node = (ngx_js_dict_node_t *) rn;
-
         kv = JS_NewArray(cx);
         if (JS_IsException(kv)) {
             ngx_rwlock_unlock(&dict->sh->rwlock);
@@ -3638,14 +3661,15 @@ ngx_qjs_ext_shared_dict_keys(JSContext *cx, JSValueConst this_val, int argc,
 
     rbtree = &dict->sh->rbtree;
 
-    ngx_rwlock_rlock(&dict->sh->rwlock);
+    now = 0;
 
     if (dict->timeout) {
         tp = ngx_timeofday();
         now = tp->sec * 1000 + tp->msec;
-        ngx_js_dict_expire(dict, now);
     }
 
+    ngx_rwlock_rlock(&dict->sh->rwlock);
+
     if (rbtree->root == rbtree->sentinel) {
         ngx_rwlock_unlock(&dict->sh->rwlock);
         return JS_NewArray(cx);
@@ -3663,12 +3687,16 @@ ngx_qjs_ext_shared_dict_keys(JSContext *cx, JSValueConst this_val, int argc,
          rn != NULL;
          rn = ngx_rbtree_next(rbtree, rn))
     {
+        node = (ngx_js_dict_node_t *) rn;
+
+        if (dict->timeout && now >= node->expire.key) {
+            continue;
+        }
+
         if (max_count-- == 0) {
             break;
         }
 
-        node = (ngx_js_dict_node_t *) rn;
-
         key = JS_NewStringLen(cx, (const char *) node->sn.str.data,
                               node->sn.str.len);
         if (JS_IsException(key)) {
@@ -3815,13 +3843,14 @@ static JSValue
 ngx_qjs_ext_shared_dict_size(JSContext *cx, JSValueConst this_val,
     int argc, JSValueConst *argv)
 {
-    njs_int_t           items;
-    ngx_msec_t          now;
-    ngx_time_t         *tp;
-    ngx_rbtree_t       *rbtree;
-    ngx_js_dict_t      *dict;
-    ngx_shm_zone_t     *shm_zone;
-    ngx_rbtree_node_t  *rn;
+    njs_int_t            items;
+    ngx_msec_t           now;
+    ngx_time_t          *tp;
+    ngx_rbtree_t        *rbtree;
+    ngx_js_dict_t       *dict;
+    ngx_shm_zone_t      *shm_zone;
+    ngx_rbtree_node_t   *rn;
+    ngx_js_dict_node_t  *node;
 
     shm_zone = JS_GetOpaque(this_val, NGX_QJS_CLASS_ID_SHARED_DICT);
     if (shm_zone == NULL) {
@@ -3830,14 +3859,15 @@ ngx_qjs_ext_shared_dict_size(JSContext *cx, JSValueConst this_val,
 
     dict = shm_zone->data;
 
-    ngx_rwlock_rlock(&dict->sh->rwlock);
+    now = 0;
 
     if (dict->timeout) {
         tp = ngx_timeofday();
         now = tp->sec * 1000 + tp->msec;
-        ngx_js_dict_expire(dict, now);
     }
 
+    ngx_rwlock_rlock(&dict->sh->rwlock);
+
     rbtree = &dict->sh->rbtree;
 
     if (rbtree->root == rbtree->sentinel) {
@@ -3851,6 +3881,12 @@ ngx_qjs_ext_shared_dict_size(JSContext *cx, JSValueConst this_val,
          rn != NULL;
          rn = ngx_rbtree_next(rbtree, rn))
     {
+        node = (ngx_js_dict_node_t *) rn;
+
+        if (dict->timeout && now >= node->expire.key) {
+            continue;
+        }
+
         items++;
     }