Skip to content

Fix remove cached eval scripts on engine unregister#3503

Merged
enjoy-binbin merged 5 commits into
valkey-io:unstablefrom
eifrah-aws:scripting-engine-crash-unload
Apr 27, 2026
Merged

Fix remove cached eval scripts on engine unregister#3503
enjoy-binbin merged 5 commits into
valkey-io:unstablefrom
eifrah-aws:scripting-engine-crash-unload

Conversation

@eifrah-aws

Copy link
Copy Markdown
Contributor

Remove eval script cache entries that belong to a scripting engine when that engine is unregistered. This prevents the eval cache from retaining dangling engine pointers and keeps the tracked script memory in sync after engine shutdown.

The scripting engine unregister path now invokes a new eval cleanup helper, which scans the cached scripts, drops matching entries from the LRU list and dictionary, and adjusts cache memory accounting accordingly.

  • scripting engine
  • eval cache

Generated by CodeLite

Remove eval script cache entries that belong to a scripting engine when that
engine is unregistered. This prevents the eval cache from retaining dangling
engine pointers and keeps the tracked script memory in sync after engine
shutdown.

The scripting engine unregister path now invokes a new eval cleanup helper,
which scans the cached scripts, drops matching entries from the LRU list and
dictionary, and adjusts cache memory accounting accordingly.

* scripting engine
* eval cache

**Generated by CodeLite**

Signed-off-by: Eran Ifrah <eifrah@amazon.com>
@eifrah-aws eifrah-aws marked this pull request as ready for review April 14, 2026 13:03
@codecov

codecov Bot commented Apr 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.43%. Comparing base (b2e0f63) to head (8c78b64).

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #3503      +/-   ##
============================================
- Coverage     76.45%   76.43%   -0.02%     
============================================
  Files           159      159              
  Lines         79840    79853      +13     
============================================
- Hits          61042    61036       -6     
- Misses        18798    18817      +19     
Files with missing lines Coverage Δ
src/eval.c 91.78% <100.00%> (+0.27%) ⬆️
src/scripting_engine.c 58.27% <100.00%> (+0.60%) ⬆️

... and 24 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/eval.c Outdated
@eifrah-aws eifrah-aws requested a review from dvkashapov April 16, 2026 05:34
@enjoy-binbin enjoy-binbin requested a review from rjd15372 April 16, 2026 11:17

@dvkashapov dvkashapov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, good catch!

@rjd15372 rjd15372 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this. LGTM

Comment thread src/scripting_engine.c Outdated
@eifrah-aws eifrah-aws requested a review from enjoy-binbin April 26, 2026 08:55

@enjoy-binbin enjoy-binbin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@enjoy-binbin enjoy-binbin moved this to To be backported in Valkey 9.0 Apr 27, 2026
@enjoy-binbin enjoy-binbin moved this to To be backported in Valkey 9.1 Apr 27, 2026
@enjoy-binbin enjoy-binbin changed the title Fix Remove Cached Eval Scripts On Engine Unregister Fix remove cached eval scripts on engine unregister Apr 27, 2026
@enjoy-binbin enjoy-binbin merged commit 6dbb7f8 into valkey-io:unstable Apr 27, 2026
55 of 56 checks passed
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/valkey that referenced this pull request Apr 27, 2026
Remove eval script cache entries that belong to a scripting engine when
that engine is unregistered. This prevents the eval cache from retaining
dangling engine pointers and keeps the tracked script memory in sync
after engine shutdown.

The scripting engine unregister path now invokes a new eval cleanup
helper, which scans the cached scripts, drops matching entries from the
LRU list and dictionary, and adjusts cache memory accounting accordingly.

* scripting engine
* eval cache

Signed-off-by: Eran Ifrah <eifrah@amazon.com>
sarthakaggarwal97 added a commit to sarthakaggarwal97/valkey that referenced this pull request Apr 27, 2026
madolson pushed a commit that referenced this pull request Apr 27, 2026
Remove eval script cache entries that belong to a scripting engine when
that engine is unregistered. This prevents the eval cache from retaining
dangling engine pointers and keeps the tracked script memory in sync
after engine shutdown.

The scripting engine unregister path now invokes a new eval cleanup
helper, which scans the cached scripts, drops matching entries from the
LRU list and dictionary, and adjusts cache memory accounting accordingly.

* scripting engine
* eval cache

Signed-off-by: Eran Ifrah <eifrah@amazon.com>
sarthakaggarwal97 added a commit to sarthakaggarwal97/valkey that referenced this pull request Apr 27, 2026
…sers)

Removed valkey-io#3504, valkey-io#3545, valkey-io#3503 - these fix bugs introduced by valkey-io#3324, valkey-io#2936,

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
valkey-io#3392 respectively, all new in RC2. Users never experienced these bugs.
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/valkey that referenced this pull request May 7, 2026
Remove eval script cache entries that belong to a scripting engine when
that engine is unregistered. This prevents the eval cache from retaining
dangling engine pointers and keeps the tracked script memory in sync
after engine shutdown.

The scripting engine unregister path now invokes a new eval cleanup
helper, which scans the cached scripts, drops matching entries from the
LRU list and dictionary, and adjusts cache memory accounting accordingly.

* scripting engine
* eval cache

Signed-off-by: Eran Ifrah <eifrah@amazon.com>
@eifrah-aws eifrah-aws deleted the scripting-engine-crash-unload branch May 11, 2026 08:25
@sarthakaggarwal97 sarthakaggarwal97 moved this from To be backported to Done in Valkey 9.1 May 16, 2026
valkeyrie-ops Bot pushed a commit that referenced this pull request May 19, 2026
Remove eval script cache entries that belong to a scripting engine when
that engine is unregistered. This prevents the eval cache from retaining
dangling engine pointers and keeps the tracked script memory in sync
after engine shutdown.

The scripting engine unregister path now invokes a new eval cleanup
helper, which scans the cached scripts, drops matching entries from the
LRU list and dictionary, and adjusts cache memory accounting accordingly.

* scripting engine
* eval cache

Signed-off-by: Eran Ifrah <eifrah@amazon.com>
@eifrah-aws eifrah-aws restored the scripting-engine-crash-unload branch June 1, 2026 07:36
@eifrah-aws eifrah-aws deleted the scripting-engine-crash-unload branch June 1, 2026 07:37
valkeyrie-ops Bot pushed a commit that referenced this pull request Jun 12, 2026
Remove eval script cache entries that belong to a scripting engine when
that engine is unregistered. This prevents the eval cache from retaining
dangling engine pointers and keeps the tracked script memory in sync
after engine shutdown.

The scripting engine unregister path now invokes a new eval cleanup
helper, which scans the cached scripts, drops matching entries from the
LRU list and dictionary, and adjusts cache memory accounting accordingly.

* scripting engine
* eval cache

Signed-off-by: Eran Ifrah <eifrah@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To be backported
Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants