From 983427973b96a3c6293a6aa6ace7c859b28f35aa Mon Sep 17 00:00:00 2001 From: Dmitry Volyntsev Date: Thu, 26 Mar 2026 17:16:25 -0700 Subject: [PATCH] Modules: improved shared dict eviction strategy. 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 --- nginx/ngx_js_shared_dict.c | 35 ++++++++++++++++++++++++-------- nginx/t/js_shared_dict_evict.t | 37 ++++++++++++++++++++++++++++++++-- 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/nginx/ngx_js_shared_dict.c b/nginx/ngx_js_shared_dict.c index b4a09bb1..54af5028 100644 --- 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,9 +1796,10 @@ 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; @@ -1794,15 +1807,17 @@ ngx_js_dict_evict(ngx_js_dict_t *dict, ngx_int_t count) 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 index 3501c238..fe0b381b 100644 --- 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'); + ############################################################################### -- 2.47.3