Fix Lua allocator corruption from disposing a session mid-script

[Tiago Nápoli (tiagonapoli)]

Problem

A session's Lua state is closed (lua_close, which frees the entire Lua heap) from SessionScriptCache.Clear() during RespServerSession / GarnetServer disposal. This can execute on a different thread than the one running a script for that session.

The clearest trigger is server shutdown: GarnetServer.Dispose() -> GarnetServerBase.DisposeActiveHandlers() -> RespServerSession.Dispose() -> SessionScriptCache.Clear() -> LuaRunner.Dispose() -> lua_close() runs on the disposing thread, while a .NET ThreadPool network worker may still be inside RunForSession for an in-flight EVAL/EVALSHA.

The Lua allocators (LuaManagedAllocator / LuaLimitedManagedAllocator) are single-threaded by design. Freeing the heap (lua_close) while a script is still allocating on the same allocator corrupts its free list. The corruption then surfaces as unrelated, hard-to-diagnose failures (scrambled interned strings, bad ACL/RESP argument parsing, Bit/Struct errors, etc.).

Root cause evidence

Reproduced deterministically under CPU-throttled containers with allocator-integrity instrumentation. Every capture showed the same two threads on the same allocator:

• NUnit / disposer thread in lua_close (SessionScriptCache.Clear), and
• a .NET TP Worker in LuaRunner.RunForSession (TryEVAL),

racing on the free list. LuaStateWrapper.Dispose already serializes lua_close with hook-setting via stateUpdateLock and is idempotent, so the timeout thread and double-dispose are not the culprit - the only unguarded conflict is script execution vs. state close.

This is the source of the flaky LuaScriptTests failures, and is also a genuine (if lower-severity) production shutdown-time race.

Fix

Add a small interlocked gate in SessionScriptCache, scoped to script execution only (zero cost for non-Lua commands; one interlocked increment/decrement per script eval):

• StartRunningScript increments an in-flight counter and returns false (refusing to start) once disposal has begun.
• StopRunningScript decrements it.
• Clear sets a disposing flag and drains in-flight executions (waits for the counter to reach 0) before disposing any runner.
• The Remove-on-failure path is kept inside the guarded window (via try/finally), so it also cannot race Clear on the runner dictionary.

Full store-load fencing via Interlocked on both sides guarantees a started execution and an in-progress disposal can never both proceed.

Testing

• dotnet build libs/server/Garnet.server.csproj -c Release — clean (0 warnings).
• Draft: still validating against the full scripting suite / repro fleet.

[Copilot]

Pull request overview

This PR addresses a shutdown-time (and disposal-time) race where a session’s Lua state can be closed (lua_close) from a different thread while a script is still allocating, corrupting the Lua allocator and causing flaky/unrelated failures. The change introduces a per-session “in-flight script” gate and updates EVAL/EVALSHA execution flow, plus adds a regression test to reproduce the race.

Changes:

• Add a per-session execution gate in SessionScriptCache so disposal can block until active script executions finish before disposing Lua runners/state.
• Refactor EVAL/EVALSHA paths to execute scripts via a single guarded helper that also keeps the “remove runner on failure” path inside the guarded window.
• Add a scripting test that disposes the server mid-script to exercise the race (and make teardown resilient to server = null).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
test/standalone/Garnet.test.scripting/LuaScriptTests.cs	Adds a regression test that disposes the server during a long-running allocation-heavy Lua script; makes teardown null-safe.
libs/server/Lua/SessionScriptCache.cs	Introduces a per-session in-flight execution monitor and uses it to drain executions before disposing cached runners/state.
libs/server/Lua/LuaCommands.cs	Refactors EVAL/EVALSHA to run scripts through a single guarded helper that pairs Start/Stop and removes cache entries on failure within the guarded region.

Comments suppressed due to low confidence (1)

libs/server/Lua/SessionScriptCache.cs:260

• Calling ActiveWorkerMonitor.Dispose() here can race a late StartRunningScript/TryEnter and throw ObjectDisposedException (ManualResetEventSlim was disposed). Also, switching to TryClose() directly is not idempotent (repeated calls can overflow). Prefer closing/draining the monitor once (guarded by a flag) without disposing the underlying event while other threads may still attempt entry.

            // Prevent new script executions and block until any in-flight script has exited before
            // disposing runners (which frees the Lua state via lua_close). This avoids corrupting the
            // single-threaded Lua allocator by freeing it while a script is still executing.
            scriptRunMonitor.Dispose();