Modules: improved shared dict eviction strategy.

xeioex · xeioex · commit 983427973b96 · 2026-03-30T16:53:46.000-07:00
Previously, when a slab allocation failed in evict mode, only 16
entries were evicted with a single retry.  This could still result
in SharedMemoryError when the freed slab slots did not match the
requested allocation size class, even though the zone had plenty
of evictable entries.

In practice, it might happen when the following conditions are met:
    - The shared zone is full
    - evict flag is enabled
    - key/value entries differ in size

The allocation now retries in a loop, evicting 16 entries at a time,
until the allocation succeeds or no more entries remain in the expire
tree.

After this change, allocation with evict enabled can only fail when:
    - the value is larger than the zone's usable space
    - the expire tree has no entries left to evict
    - zone metadata overhead leaves insufficient room
diff --git a/nginx/ngx_js_shared_dict.c b/nginx/ngx_js_shared_dict.c
@@ -128,7 +128,7 @@ static ngx_int_t ngx_js_dict_copy_value_locked(njs_vm_t *vm,
     ngx_js_dict_t *dict, ngx_js_dict_node_t *node, njs_value_t *retval);
 
 static void ngx_js_dict_expire(ngx_js_dict_t *dict, ngx_msec_t now);
-static void ngx_js_dict_evict(ngx_js_dict_t *dict, ngx_int_t count);
+static ngx_uint_t ngx_js_dict_evict(ngx_js_dict_t *dict, ngx_uint_t count);
 
 static njs_int_t ngx_js_dict_shared_error_name(njs_vm_t *vm,
     njs_object_prop_t *prop, uint32_t unused, njs_value_t *value,
@@ -1371,12 +1371,24 @@ ngx_js_dict_alloc(ngx_js_dict_t *dict, size_t n)
 {
     void  *p;
 
-    dict->shpool->log_nomem = !dict->evict;
+    if (!dict->evict) {
+        return ngx_slab_alloc_locked(dict->shpool, n);
+    }
+
+    dict->shpool->log_nomem = 0;
     p = ngx_slab_alloc_locked(dict->shpool, n);
+
+    while (p == NULL) {
+        if (ngx_js_dict_evict(dict, 16) == 0) {
+            break;
+        }
+
+        p = ngx_slab_alloc_locked(dict->shpool, n);
+    }
+
     dict->shpool->log_nomem = 1;
 
-    if (p == NULL && dict->evict) {
-        ngx_js_dict_evict(dict, 16);
+    if (p == NULL) {
         p = ngx_slab_alloc_locked(dict->shpool, n);
     }
 
@@ -1784,25 +1796,28 @@ ngx_js_dict_expire(ngx_js_dict_t *dict, ngx_msec_t now)
 }
 
 
-static void
-ngx_js_dict_evict(ngx_js_dict_t *dict, ngx_int_t count)
+static ngx_uint_t
+ngx_js_dict_evict(ngx_js_dict_t *dict, ngx_uint_t count)
 {
+    ngx_uint_t           evicted;
     ngx_rbtree_t        *rbtree;
     ngx_rbtree_node_t   *rn, *next;
     ngx_js_dict_node_t  *node;
 
     rbtree = &dict->sh->rbtree_expire;
 
     if (rbtree->root == rbtree->sentinel) {
-        return;
+        return 0;
     }
 
+    evicted = 0;
+
     for (rn = ngx_rbtree_min(rbtree->root, rbtree->sentinel);
          rn != NULL;
          rn = next)
     {
         if (count-- == 0) {
-            return;
+            return evicted;
         }
 
         node = (ngx_js_dict_node_t *)
@@ -1815,7 +1830,11 @@ ngx_js_dict_evict(ngx_js_dict_t *dict, ngx_int_t count)
         ngx_rbtree_delete(&dict->sh->rbtree, (ngx_rbtree_node_t *) node);
 
         ngx_js_dict_node_free(dict, node);
+
+        evicted++;
     }
+
+    return evicted;
 }
 
 
diff --git a/nginx/t/js_shared_dict_evict.t b/nginx/t/js_shared_dict_evict.t
@@ -51,6 +51,10 @@ http {
         location /no_self_evict {
             js_content test.no_self_evict;
         }
+
+        location /cross_slab_class {
+            js_content test.cross_slab_class;
+        }
     }
 }
 
@@ -110,12 +114,38 @@ $t->write_file('test.js', <<'EOF');
         r.return(200, 'FILLED:' + elems);
     }
 
-    export default { stress, no_self_evict };
+    function cross_slab_class(r) {
+        var dict = ngx.shared.stress;
+        var v = 'x'.repeat(16);
+
+        /* Calibrate: find how many tiny entries fit. */
+        dict.clear();
+        dict.set('probe', v);
+
+        var elems = 0;
+        while (dict.has('probe')) {
+            dict.set('s_' + elems++, v);
+        }
+
+        /* Rebuild at exact capacity with tiny entries. */
+        dict.clear();
+        for (var i = 0; i < elems; i++) {
+            dict.set('s_' + i, v);
+        }
+
+        /* Check that the zone can evict enough entries to fit a big one. */
+
+        dict.set('big', 'y'.repeat(2048));
+
+        r.return(200, 'FILLED:' + elems);
+    }
+
+    export default { stress, no_self_evict, cross_slab_class };
 EOF
 
 $t->try_run('no js_shared_dict_zone');
 
-$t->plan(6);
+$t->plan(7);
 
 ###############################################################################
 
@@ -131,4 +161,7 @@ like($update_resp, qr/FILLED:/, 'evict update: zone filled');
 unlike($t->read_file('error.log'), qr/is already free/,
 	'evict update: no double-free in error log');
 
+like(http_get('/cross_slab_class'), qr/FILLED:/,
+	'evict cross slab class: large alloc after small entries');
+
 ###############################################################################
