From 77ea719c3173bb6648a726348440de9096609bb4 Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Tue, 15 Sep 2026 14:34:19 -0700 Subject: [PATCH 01/27] Reject untrusted Lua bytecode Keep application bytecode caching while requiring customer input to compile in text-only mode and tagging internally generated chunks for binary-only loading. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- .../Lua/LuaScriptCacheOperations.cs | 7 +- libs/server/Lua/LuaCommands.cs | 18 ++-- libs/server/Lua/LuaRunner.Functions.cs | 18 ++-- libs/server/Lua/LuaRunner.Loader.cs | 36 +++++--- libs/server/Lua/LuaRunner.cs | 20 ++++- libs/server/Lua/LuaScriptHandle.cs | 25 +++++- libs/server/Lua/LuaStateWrapper.cs | 27 ++---- libs/server/Lua/NativeMethods.cs | 27 +++--- libs/server/Lua/SessionScriptCache.cs | 82 ++++++++++++++++++- .../LuaScriptRunnerTests.cs | 19 +++++ .../Garnet.test.scripting/LuaScriptTests.cs | 50 +++++++++++ 11 files changed, 265 insertions(+), 64 deletions(-) diff --git a/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs b/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs index 72558d01f1c..de4f382a65a 100644 --- a/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs +++ b/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs @@ -82,7 +82,7 @@ public void IterationSetup() // Make outer hit available for every iteration LuaScriptHandle scriptHandle = null; - if (!sessionScriptCache.TryLoad(session, "return 1"u8, new(outerHitDigest), ref scriptHandle, out _, out _)) + if (!sessionScriptCache.TryLoadSource(session, "return 1"u8, new(outerHitDigest), ref scriptHandle, out _, out _)) { throw new InvalidOperationException("Should have been able to load"); } @@ -147,10 +147,9 @@ private void LoadScript(Span digest) { if (storeWrapper.storeScriptCache.TryGetValue(digestKey, out var scriptHandle)) { - LuaScriptHandle newScriptHandle = null; - if (!sessionScriptCache.TryLoad(session, scriptHandle.ScriptData.Span, digestKey, ref newScriptHandle, out runner, out _)) + if (!sessionScriptCache.TryLoadCached(session, digestKey, ref scriptHandle, out runner, out _)) { - // TryLoad will have written an error out, it any + // TryLoadCached will have written an error out, if any _ = storeWrapper.storeScriptCache.TryRemove(digestKey, out _); } diff --git a/libs/server/Lua/LuaCommands.cs b/libs/server/Lua/LuaCommands.cs index 4a37d88e902..bb3106454ea 100644 --- a/libs/server/Lua/LuaCommands.cs +++ b/libs/server/Lua/LuaCommands.cs @@ -48,9 +48,9 @@ private unsafe bool TryEVALSHA() { if (storeWrapper.storeScriptCache.TryGetValue(scriptKey, out var globalScriptHandle)) { - if (!sessionScriptCache.TryLoad(this, globalScriptHandle.ScriptData.Span, scriptKey, ref globalScriptHandle, out runner, out _)) + if (!sessionScriptCache.TryLoadCached(this, scriptKey, ref globalScriptHandle, out runner, out _)) { - // TryLoad will have written an error out, it any + // TryLoadCached will have written an error out, if any // // Note we DON'T dispose the script handle because this is just the session cache _ = storeWrapper.storeScriptCache.TryRemove(scriptKey, out _); @@ -118,9 +118,9 @@ private unsafe bool TryEVAL() var sessionScriptHandle = globalScriptHandle; - if (!sessionScriptCache.TryLoad(this, script.ReadOnlySpan, onStackScriptKey, ref sessionScriptHandle, out var runner, out var digestOnHeap)) + if (!sessionScriptCache.TryLoadSource(this, script.ReadOnlySpan, onStackScriptKey, ref sessionScriptHandle, out var runner, out var digestOnHeap)) { - // TryLoad will have written any errors out + // TryLoadSource will have written any errors out return true; } else if (sessionScriptHandle != globalScriptHandle) @@ -278,9 +278,9 @@ private bool NetworkScriptLoad() _ = storeWrapper.storeScriptCache.TryGetValue(onStackScriptHashKey, out var globalScriptHandle); var sessionScriptHandle = globalScriptHandle; - if (sessionScriptCache.TryLoad(this, source.ReadOnlySpan, onStackScriptHashKey, ref sessionScriptHandle, out _, out var digestOnHeap)) + if (sessionScriptCache.TryLoadSource(this, source.ReadOnlySpan, onStackScriptHashKey, ref sessionScriptHandle, out _, out var digestOnHeap)) { - // TryLoad will write any errors out + // TryLoadSource will write any errors out // Add script to the global store dictionary if not already in there if (globalScriptHandle != sessionScriptHandle) @@ -332,6 +332,12 @@ private bool CheckLuaEnabled() return true; } + internal void WriteLuaCompilationError(string error) + { + while (!RespWriteUtils.TryWriteError($"Compilation error: {error}", ref dcurr, dend)) + SendAndReset(); + } + /// /// Run a resolved script for the current session. /// diff --git a/libs/server/Lua/LuaRunner.Functions.cs b/libs/server/Lua/LuaRunner.Functions.cs index 23a6295df57..dc02f30e06f 100644 --- a/libs/server/Lua/LuaRunner.Functions.cs +++ b/libs/server/Lua/LuaRunner.Functions.cs @@ -852,7 +852,7 @@ internal int LoadString(nint luaStatePtr) return LuaWrappedError(1, constStrs.InsufficientLuaStackSpace); } - var res = state.LoadString(buff); + var res = state.LoadTextBuffer(buff); if (res != LuaStatus.OK) { state.ClearStack(); @@ -3071,8 +3071,8 @@ int luaArgCount private unsafe int CompileCommon(nint luaState, ref TResponse resp) where TResponse : struct, IResponseAdapter { - // 1 for function, 1 for code string - const int NeededStackSpace = 2; + // 1 for function, 1 for code string, 1 for load mode + const int NeededStackSpace = 3; Debug.Assert(functionRegistryIndex == -1, "Shouldn't compile multiple times"); @@ -3081,7 +3081,15 @@ private unsafe int CompileCommon(nint luaState, ref TResponse resp) Debug.Assert(state.TryEnsureMinimumStackCapacity(NeededStackSpace), "LUA_MIN_STACK should be high enough that this cannot happen"); _ = state.RawGetInteger(LuaType.Function, (int)LuaRegistry.Index, loadSandboxedRegistryIndex); - if (!state.TryPushBuffer(source.Span)) + if (!state.TryPushBuffer(source.Data.Span)) + { + while (!RespWriteUtils.TryWriteError(CmdStrings.LUA_out_of_memory, ref resp.BufferCur, resp.BufferEnd)) + resp.SendAndReset(); + + return 0; + } + + if (!state.TryPushBuffer(source.Kind == LuaScriptChunkKind.GarnetGeneratedBinary ? "b"u8 : "t"u8)) { while (!RespWriteUtils.TryWriteError(CmdStrings.LUA_out_of_memory, ref resp.BufferCur, resp.BufferEnd)) resp.SendAndReset(); @@ -3089,7 +3097,7 @@ private unsafe int CompileCommon(nint luaState, ref TResponse resp) return 0; } - var callRes = state.PCall(1, 2); + var callRes = state.PCall(2, 2); // On success the stack will have two things on it: // 1. The error (nil if not error) diff --git a/libs/server/Lua/LuaRunner.Loader.cs b/libs/server/Lua/LuaRunner.Loader.cs index a9a95ef9e53..59751f19cdc 100644 --- a/libs/server/Lua/LuaRunner.Loader.cs +++ b/libs/server/Lua/LuaRunner.Loader.cs @@ -314,8 +314,8 @@ function reset_keys_and_argv(fromKey, fromArgv) -- force new 'global' environment to be readonly recursively_readonly_table(sandbox_env) -- responsible for sandboxing user provided code -function load_sandboxed(source) - local rawFunc, err = load(source, nil, nil, sandbox_env) +function load_sandboxed(source, mode) + local rawFunc, err = load(source, nil, mode, sandbox_env) return err, rawFunc end @@ -448,7 +448,7 @@ internal static ReadOnlyMemory PrepareLoaderBlockBytes(HashSet all compilingState.Remove(1); - if (compilingState.LoadString(Encoding.UTF8.GetBytes(finalLoaderBlock)) != LuaStatus.OK) + if (compilingState.LoadTextBuffer(Encoding.UTF8.GetBytes(finalLoaderBlock)) != LuaStatus.OK) { throw new InvalidOperationException("Compiling function should not fail"); } @@ -479,7 +479,7 @@ internal static ReadOnlyMemory PrepareLoaderBlockBytes(HashSet all /// /// These ops are faster to load into a runtime than parsing the whole source file again. /// - internal static byte[] CompileSource(ReadOnlySpan source) + internal static bool TryCompileSource(ReadOnlySpan source, out LuaScriptChunk compiledSource, out string error) { // This is equivalent to calling // @@ -496,23 +496,39 @@ internal static byte[] CompileSource(ReadOnlySpan source) state.Remove(1); - if (state.LoadString(source) != LuaStatus.OK) + if (state.LoadTextBuffer(source) != LuaStatus.OK) { - // If we're going to fail, just keep the source as is - a future load attempt will fail it too - return source.ToArray(); + compiledSource = default; + error = GetError(state); + return false; } state.PushBoolean(true); if (state.PCall(2, 1) != LuaStatus.OK) { - // If we're going to fail, just keep the source as is - a future load attempt will fail it too - return source.ToArray(); + compiledSource = default; + error = GetError(state); + return false; } state.KnownStringToBuffer(1, out var ops); - return ops.ToArray(); + compiledSource = new(ops.ToArray(), LuaScriptChunkKind.GarnetGeneratedBinary); + error = null; + return true; + + static string GetError(LuaStateWrapper state) + { + var errorIndex = state.StackTop; + if (errorIndex >= 1 && state.Type(errorIndex) == LuaType.String) + { + state.KnownStringToBuffer(errorIndex, out var errorBuffer); + return Encoding.UTF8.GetString(errorBuffer); + } + + return "cause unknown"; + } } } } \ No newline at end of file diff --git a/libs/server/Lua/LuaRunner.cs b/libs/server/Lua/LuaRunner.cs index 5b68363b7af..10ec29db641 100644 --- a/libs/server/Lua/LuaRunner.cs +++ b/libs/server/Lua/LuaRunner.cs @@ -160,7 +160,7 @@ public void SendAndReset() readonly LuaLoggingMode logMode; readonly HashSet allowedFunctions; - readonly ReadOnlyMemory source; + readonly LuaScriptChunk source; readonly ScratchBufferNetworkSender scratchBufferNetworkSender; readonly RespServerSession respServerSession; @@ -214,6 +214,22 @@ public unsafe LuaRunner( ScratchBufferNetworkSender scratchBufferNetworkSender = null, string redisVersion = "0.0.0.0", ILogger logger = null + ) + : this(memMode, memLimitBytes, logMode, allowedFunctions, new LuaScriptChunk(source, LuaScriptChunkKind.Text), txnMode, respServerSession, scratchBufferNetworkSender, redisVersion, logger) + { + } + + internal unsafe LuaRunner( + LuaMemoryManagementMode memMode, + int? memLimitBytes, + LuaLoggingMode logMode, + HashSet allowedFunctions, + LuaScriptChunk source, + bool txnMode = false, + RespServerSession respServerSession = null, + ScratchBufferNetworkSender scratchBufferNetworkSender = null, + string redisVersion = "0.0.0.0", + ILogger logger = null ) { // KEYS and ARGV are always access by index, and to avoid allocation concerns @@ -328,7 +344,7 @@ public unsafe LuaRunner( throw new GarnetException("Insufficient space in Lua VM for redis version number global"); } - var loadRes = state.LoadBuffer(PrepareLoaderBlockBytes(allowedFunctions, logger).Span); + var loadRes = state.LoadBinaryBuffer(PrepareLoaderBlockBytes(allowedFunctions, logger).Span); if (loadRes != LuaStatus.OK) { if (state.StackTop == 1 && state.Type(1) == LuaType.String) diff --git a/libs/server/Lua/LuaScriptHandle.cs b/libs/server/Lua/LuaScriptHandle.cs index caa9f066dfa..196ac66817b 100644 --- a/libs/server/Lua/LuaScriptHandle.cs +++ b/libs/server/Lua/LuaScriptHandle.cs @@ -5,6 +5,14 @@ namespace Garnet.server { + internal enum LuaScriptChunkKind : byte + { + Text, + GarnetGeneratedBinary + } + + internal readonly record struct LuaScriptChunk(ReadOnlyMemory Data, LuaScriptChunkKind Kind); + /// /// Used to track the lifetime a shared Lua script, which may end up backing multiple s. /// @@ -19,13 +27,24 @@ public sealed class LuaScriptHandle : IDisposable public bool IsDisposed { get; private set; } /// - /// Source (or compiled source) for the associated Lua script. + /// Source or internally compiled data for the associated Lua script. /// - public ReadOnlyMemory ScriptData { get; } + public ReadOnlyMemory ScriptData => Chunk.Data; + + internal LuaScriptChunk Chunk { get; } + /// + /// Creates a handle for Lua source text. + /// + /// Lua source text. public LuaScriptHandle(ReadOnlyMemory scriptData) + : this(new LuaScriptChunk(scriptData, LuaScriptChunkKind.Text)) + { + } + + internal LuaScriptHandle(LuaScriptChunk chunk) { - ScriptData = scriptData; + Chunk = chunk; } /// diff --git a/libs/server/Lua/LuaStateWrapper.cs b/libs/server/Lua/LuaStateWrapper.cs index 48f9fca6203..4ecfa34ed65 100644 --- a/libs/server/Lua/LuaStateWrapper.cs +++ b/libs/server/Lua/LuaStateWrapper.cs @@ -456,20 +456,14 @@ internal bool TrySetGlobal(ReadOnlySpan nullTerminatedGlobalName) } /// - /// This should be used for all LoadBuffers into Lua. - /// - /// Note that this is different from pushing a buffer, as the loaded buffer is compiled and executed. - /// - /// Maintains and to minimize p/invoke calls. + /// Load Garnet-generated bytecode into Lua. /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal LuaStatus LoadBuffer(ReadOnlySpan buffer) + internal LuaStatus LoadBinaryBuffer(ReadOnlySpan buffer) { AssertLuaStackNotFull(2); - // Note that https://www.lua.org/source/5.4/lauxlib.c.html#luaL_loadbufferx is implemented in terms of - // a PCall, so we don't have to worry about crashes. - var ret = NativeMethods.LoadBuffer(state, buffer); + var ret = NativeMethods.LoadBinaryBuffer(state, buffer); if (ret != LuaStatus.OK) { @@ -486,19 +480,16 @@ internal LuaStatus LoadBuffer(ReadOnlySpan buffer) } /// - /// This should be used for all LoadStrings into Lua. - /// - /// Note that this is different from pushing or loading buffer, as the loaded buffer is compiled but NOT executed. - /// - /// Maintains and to minimize p/invoke calls. + /// This should be used for compiling untrusted Lua source text. + /// + /// Binary chunks are rejected by the Lua runtime. /// - internal LuaStatus LoadString(ReadOnlySpan buffer) + internal LuaStatus LoadTextBuffer(ReadOnlySpan buffer) { AssertLuaStackNotFull(2); - // Note that https://www.lua.org/source/5.4/lauxlib.h.html#luaL_loadbuffer is implemented in terms of - // a PCall, so we don't have to worry about crashes. - var ret = NativeMethods.LoadString(state, buffer); + // Text-only mode rejects binary chunks at the untrusted input boundary. + var ret = NativeMethods.LoadTextBuffer(state, buffer); if (ret != LuaStatus.OK) { diff --git a/libs/server/Lua/NativeMethods.cs b/libs/server/Lua/NativeMethods.cs index 254b6c43e88..1cbfede447e 100644 --- a/libs/server/Lua/NativeMethods.cs +++ b/libs/server/Lua/NativeMethods.cs @@ -45,13 +45,6 @@ internal static partial class NativeMethods [UnmanagedCallConv(CallConvs = [typeof(CallConvCdecl)])] private static partial LuaStatus luaL_loadbufferx(lua_State luaState, charptr_t buff, size_t sz, charptr_t name, charptr_t mode); - /// - /// see: https://www.lua.org/manual/5.4/manual.html#luaL_loadstring - /// - [LibraryImport(LuaLibraryName)] - [UnmanagedCallConv(CallConvs = [typeof(CallConvCdecl)])] - private static partial LuaStatus luaL_loadstring(lua_State lua_State, charptr_t buff); - /// /// see: https://www.lua.org/manual/5.4/manual.html#luaL_newstate /// @@ -388,28 +381,34 @@ internal static unsafe void PushBuffer(lua_State luaState, ReadOnlySpan st } /// - /// Push given span to stack, compiles it, and executes it. + /// Load Garnet-generated bytecode. /// /// Provided data is copied, and can be reused once this call returns. /// - internal static unsafe LuaStatus LoadBuffer(lua_State luaState, ReadOnlySpan str) + internal static unsafe LuaStatus LoadBinaryBuffer(lua_State luaState, ReadOnlySpan str) { + // Binary-only mode is reserved for bytecode generated internally by Garnet. + ReadOnlySpan mode = "b\0"u8; fixed (byte* ptr = str) + fixed (byte* modePtr = mode) { - return luaL_loadbufferx(luaState, (charptr_t)ptr, (size_t)str.Length, (charptr_t)UIntPtr.Zero, (charptr_t)UIntPtr.Zero); + return luaL_loadbufferx(luaState, (charptr_t)ptr, (size_t)str.Length, (charptr_t)UIntPtr.Zero, (charptr_t)modePtr); } } /// - /// Push given span to stack, and compiles it. - /// + /// Push given text span to stack and compile it. + /// /// Provided data is copied, and can be reused once this call returns. /// - internal static unsafe LuaStatus LoadString(lua_State luaState, ReadOnlySpan str) + internal static unsafe LuaStatus LoadTextBuffer(lua_State luaState, ReadOnlySpan str) { + // Text-only mode rejects customer-supplied precompiled bytecode. + ReadOnlySpan mode = "t\0"u8; fixed (byte* ptr = str) + fixed (byte* modePtr = mode) { - return luaL_loadstring(luaState, (charptr_t)ptr); + return luaL_loadbufferx(luaState, (charptr_t)ptr, (size_t)str.Length, (charptr_t)UIntPtr.Zero, (charptr_t)modePtr); } } diff --git a/libs/server/Lua/SessionScriptCache.cs b/libs/server/Lua/SessionScriptCache.cs index 705a0b9deaf..82b98b215c4 100644 --- a/libs/server/Lua/SessionScriptCache.cs +++ b/libs/server/Lua/SessionScriptCache.cs @@ -167,7 +167,7 @@ public bool TryGetFromDigest(ScriptHashKey digest, out LuaRunner scriptRunner, o /// /// If necessary, will be set so the allocation can be reused. /// - internal bool TryLoad( + internal bool TryLoadSource( RespServerSession session, ReadOnlySpan source, ScriptHashKey digest, @@ -183,10 +183,88 @@ out ScriptHashKey? digestOnHeap return true; } + if (luaScriptHandle != null) + return TryLoadCached(session, digest, ref luaScriptHandle, out runner, out digestOnHeap); + + return TryCompileAndLoad(session, source, digest, ref luaScriptHandle, out runner, out digestOnHeap); + } + + /// + /// Load a script previously stored in the global cache. + /// + internal bool TryLoadCached( + RespServerSession session, + ScriptHashKey digest, + ref LuaScriptHandle luaScriptHandle, + out LuaRunner runner, + out ScriptHashKey? digestOnHeap + ) + { + if (TryGetFromDigest(digest, out runner, out var existingLuaScriptHandle)) + { + luaScriptHandle = existingLuaScriptHandle; + digestOnHeap = null; + return true; + } + + if (luaScriptHandle.Chunk.Kind == LuaScriptChunkKind.GarnetGeneratedBinary) + return TryLoadCompiled(session, luaScriptHandle.Chunk, digest, ref luaScriptHandle, out runner, out digestOnHeap); + + return TryCompileAndLoad(session, luaScriptHandle.ScriptData.Span, digest, ref luaScriptHandle, out runner, out digestOnHeap); + } + + private bool TryCompileAndLoad( + RespServerSession session, + ReadOnlySpan source, + ScriptHashKey digest, + ref LuaScriptHandle luaScriptHandle, + out LuaRunner runner, + out ScriptHashKey? digestOnHeap + ) + { + LuaScriptChunk compiledSource; + string error; try { - var compiledSource = LuaRunner.CompileSource(source); + if (LuaRunner.TryCompileSource(source, out compiledSource, out error)) + return TryLoadCompiled(session, compiledSource, digest, ref luaScriptHandle, out runner, out digestOnHeap); + } + catch (Exception ex) + { + logger?.LogError(ex, "During Lua script compilation, an unexpected exception"); + runner = null; + digestOnHeap = null; + luaScriptHandle = null; + return false; + } + + session.WriteLuaCompilationError(error); + runner = null; + digestOnHeap = null; + return false; + } + + /// + /// Load internally compiled script bytecode into the cache. + /// + private bool TryLoadCompiled( + RespServerSession session, + LuaScriptChunk compiledSource, + ScriptHashKey digest, + ref LuaScriptHandle luaScriptHandle, + out LuaRunner runner, + out ScriptHashKey? digestOnHeap + ) + { + if (TryGetFromDigest(digest, out runner, out var existingLuaScriptHandle)) + { + luaScriptHandle = existingLuaScriptHandle; + digestOnHeap = null; + return true; + } + try + { runner = new LuaRunner(memoryManagementMode, memoryLimitBytes, logMode, allowedFunctions, compiledSource, storeWrapper.serverOptions.LuaTransactionMode, processor, scratchBufferNetworkSender, storeWrapper.redisProtocolVersion, logger); // If compilation fails, an error is written out diff --git a/test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs b/test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs index cf879a46ad9..dc8a7d7e6fb 100644 --- a/test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs +++ b/test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs @@ -153,6 +153,25 @@ public void CanLoadScript() } } + [Test] + public void TryCompileSourceRejectsBinaryAndInvalidInput() + { + ClassicAssert.IsTrue(LuaRunner.TryCompileSource("return 1"u8, out var compiledSource, out var compileError)); + ClassicAssert.IsNull(compileError); + ClassicAssert.AreEqual(LuaScriptChunkKind.GarnetGeneratedBinary, compiledSource.Kind); + ClassicAssert.GreaterOrEqual(compiledSource.Data.Length, 4); + CollectionAssert.AreEqual(new byte[] { 0x1B, (byte)'L', (byte)'u', (byte)'a' }, compiledSource.Data.Span[..4].ToArray()); + + ClassicAssert.IsFalse(LuaRunner.TryCompileSource(compiledSource.Data.Span, out var rejectedBinary, out var binaryError)); + ClassicAssert.AreEqual(default(LuaScriptChunk), rejectedBinary); + StringAssert.Contains("binary chunk", binaryError); + + var invalidSource = "return )"u8; + ClassicAssert.IsFalse(LuaRunner.TryCompileSource(invalidSource, out var rejectedSource, out var sourceError)); + ClassicAssert.AreEqual(default(LuaScriptChunk), rejectedSource); + ClassicAssert.IsNotEmpty(sourceError); + } + [Test] public void CanRunScript() { diff --git a/test/standalone/Garnet.test.scripting/LuaScriptTests.cs b/test/standalone/Garnet.test.scripting/LuaScriptTests.cs index 55d3f728887..552f3d0fa65 100644 --- a/test/standalone/Garnet.test.scripting/LuaScriptTests.cs +++ b/test/standalone/Garnet.test.scripting/LuaScriptTests.cs @@ -1499,6 +1499,56 @@ public void Issue1079() ClassicAssert.AreEqual("hello lua", success); } + [Test] + public void ScriptInputsRejectPrecompiledLuaBytecode() + { + const string Key = "binary-chunk-key"; + const string Script = "return string.dump(function() return redis.call('SET', KEYS[1], 'binary-chunk-executed') end, true)"; + + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(); + + var binaryChunk = (byte[])db.ScriptEvaluate(Script); + ClassicAssert.GreaterOrEqual(binaryChunk.Length, 4); + CollectionAssert.AreEqual(new byte[] { 0x1B, (byte)'L', (byte)'u', (byte)'a' }, binaryChunk.AsSpan(0, 4).ToArray()); + + var exception = ClassicAssert.Throws(() => db.Execute("EVAL", [binaryChunk, 1, Key])); + StringAssert.Contains("binary chunk", exception.Message); + ClassicAssert.IsFalse(db.KeyExists(Key)); + + var hash = Convert.ToHexString(SHA1.HashData(binaryChunk)).ToLowerInvariant(); + exception = ClassicAssert.Throws(() => db.Execute("SCRIPT", ["LOAD", binaryChunk])); + StringAssert.Contains("binary chunk", exception.Message); + + var exists = (RedisResult[])db.Execute("SCRIPT", ["EXISTS", hash]); + ClassicAssert.AreEqual(0, (int)exists[0]); + } + + [Test] + public void HostInsertedScriptSourceIsCompiledAsText() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(); + var source = "return 2"u8; + var hash = Convert.ToHexString(SHA1.HashData(source)).ToLowerInvariant(); + var digest = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); + _ = Encoding.ASCII.GetBytes(hash, digest); + + ClassicAssert.IsTrue(server.Provider.StoreWrapper.storeScriptCache.TryAdd(new ScriptHashKey(digest), new LuaScriptHandle(source.ToArray()))); + ClassicAssert.AreEqual(2, (int)db.Execute("EVALSHA", hash, 0)); + } + + [Test] + public void EvalUsesFullSourceLength() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(); + var source = Encoding.UTF8.GetBytes("return 1\0return 2"); + + var exception = ClassicAssert.Throws(() => db.Execute("EVAL", [source, 0])); + StringAssert.StartsWith("Compilation error:", exception.Message); + } + [TestCase(2)] [TestCase(3)] public void LuaToResp2Conversions(int redisSetRespVersion) From 91d6652101f6f46e4602677e85f0f24e56ee9d83 Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Tue, 15 Sep 2026 14:39:06 -0700 Subject: [PATCH 02/27] Clarify Lua bytecode regression tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- test/standalone/Garnet.test.scripting/LuaScriptTests.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/standalone/Garnet.test.scripting/LuaScriptTests.cs b/test/standalone/Garnet.test.scripting/LuaScriptTests.cs index 552f3d0fa65..3942216dc24 100644 --- a/test/standalone/Garnet.test.scripting/LuaScriptTests.cs +++ b/test/standalone/Garnet.test.scripting/LuaScriptTests.cs @@ -1508,14 +1508,17 @@ public void ScriptInputsRejectPrecompiledLuaBytecode() using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(); + // Generate valid bytecode matching the server's exact Lua version. var binaryChunk = (byte[])db.ScriptEvaluate(Script); ClassicAssert.GreaterOrEqual(binaryChunk.Length, 4); CollectionAssert.AreEqual(new byte[] { 0x1B, (byte)'L', (byte)'u', (byte)'a' }, binaryChunk.AsSpan(0, 4).ToArray()); + // EVAL must not execute arbitrary bytecode supplied as the script body. var exception = ClassicAssert.Throws(() => db.Execute("EVAL", [binaryChunk, 1, Key])); StringAssert.Contains("binary chunk", exception.Message); ClassicAssert.IsFalse(db.KeyExists(Key)); + // SCRIPT LOAD must reject the same bytes without adding them to the global cache. var hash = Convert.ToHexString(SHA1.HashData(binaryChunk)).ToLowerInvariant(); exception = ClassicAssert.Throws(() => db.Execute("SCRIPT", ["LOAD", binaryChunk])); StringAssert.Contains("binary chunk", exception.Message); @@ -1534,6 +1537,7 @@ public void HostInsertedScriptSourceIsCompiledAsText() var digest = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); _ = Encoding.ASCII.GetBytes(hash, digest); + // The public handle constructor accepts source text, not trusted precompiled bytecode. ClassicAssert.IsTrue(server.Provider.StoreWrapper.storeScriptCache.TryAdd(new ScriptHashKey(digest), new LuaScriptHandle(source.ToArray()))); ClassicAssert.AreEqual(2, (int)db.Execute("EVALSHA", hash, 0)); } @@ -1545,6 +1549,7 @@ public void EvalUsesFullSourceLength() var db = redis.GetDatabase(); var source = Encoding.UTF8.GetBytes("return 1\0return 2"); + // Exact-length loading must parse bytes after the NUL instead of truncating the script. var exception = ClassicAssert.Throws(() => db.Execute("EVAL", [source, 0])); StringAssert.StartsWith("Compilation error:", exception.Message); } From 74078ba53b7f52cf318e93f3eccb4dc2eedd16ae Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Tue, 15 Sep 2026 14:52:23 -0700 Subject: [PATCH 03/27] Clarify Lua compilation boundary test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs b/test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs index dc8a7d7e6fb..13e6bec9191 100644 --- a/test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs +++ b/test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs @@ -156,16 +156,19 @@ public void CanLoadScript() [Test] public void TryCompileSourceRejectsBinaryAndInvalidInput() { + // Valid source is compiled into bytecode explicitly tagged as generated by Garnet. ClassicAssert.IsTrue(LuaRunner.TryCompileSource("return 1"u8, out var compiledSource, out var compileError)); ClassicAssert.IsNull(compileError); ClassicAssert.AreEqual(LuaScriptChunkKind.GarnetGeneratedBinary, compiledSource.Kind); ClassicAssert.GreaterOrEqual(compiledSource.Data.Length, 4); CollectionAssert.AreEqual(new byte[] { 0x1B, (byte)'L', (byte)'u', (byte)'a' }, compiledSource.Data.Span[..4].ToArray()); + // Customer-provided bytecode must not be accepted as source or returned as trusted output. ClassicAssert.IsFalse(LuaRunner.TryCompileSource(compiledSource.Data.Span, out var rejectedBinary, out var binaryError)); ClassicAssert.AreEqual(default(LuaScriptChunk), rejectedBinary); StringAssert.Contains("binary chunk", binaryError); + // Invalid source must report an error without preserving the original bytes as executable data. var invalidSource = "return )"u8; ClassicAssert.IsFalse(LuaRunner.TryCompileSource(invalidSource, out var rejectedSource, out var sourceError)); ClassicAssert.AreEqual(default(LuaScriptChunk), rejectedSource); From 2508af61531eafd3b355c8030bedf4ee33fef339 Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Tue, 15 Sep 2026 14:59:50 -0700 Subject: [PATCH 04/27] Use neutral Lua input terminology Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- libs/server/Lua/NativeMethods.cs | 2 +- test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/server/Lua/NativeMethods.cs b/libs/server/Lua/NativeMethods.cs index 1cbfede447e..bd24f0b5a6b 100644 --- a/libs/server/Lua/NativeMethods.cs +++ b/libs/server/Lua/NativeMethods.cs @@ -403,7 +403,7 @@ internal static unsafe LuaStatus LoadBinaryBuffer(lua_State luaState, ReadOnlySp /// internal static unsafe LuaStatus LoadTextBuffer(lua_State luaState, ReadOnlySpan str) { - // Text-only mode rejects customer-supplied precompiled bytecode. + // Text-only mode rejects externally supplied precompiled bytecode. ReadOnlySpan mode = "t\0"u8; fixed (byte* ptr = str) fixed (byte* modePtr = mode) diff --git a/test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs b/test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs index 13e6bec9191..c43eae89f48 100644 --- a/test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs +++ b/test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs @@ -163,7 +163,7 @@ public void TryCompileSourceRejectsBinaryAndInvalidInput() ClassicAssert.GreaterOrEqual(compiledSource.Data.Length, 4); CollectionAssert.AreEqual(new byte[] { 0x1B, (byte)'L', (byte)'u', (byte)'a' }, compiledSource.Data.Span[..4].ToArray()); - // Customer-provided bytecode must not be accepted as source or returned as trusted output. + // Externally provided bytecode must not be accepted as source or returned as trusted output. ClassicAssert.IsFalse(LuaRunner.TryCompileSource(compiledSource.Data.Span, out var rejectedBinary, out var binaryError)); ClassicAssert.AreEqual(default(LuaScriptChunk), rejectedBinary); StringAssert.Contains("binary chunk", binaryError); From fde3ed915e21d55f4431188511416a745a38aa52 Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Tue, 15 Sep 2026 15:12:51 -0700 Subject: [PATCH 05/27] Simplify Lua bytecode regression setup Generate the test bytecode directly through Garnet's internal compiler instead of round-tripping through a Lua script. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- test/standalone/Garnet.test.scripting/LuaScriptTests.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/standalone/Garnet.test.scripting/LuaScriptTests.cs b/test/standalone/Garnet.test.scripting/LuaScriptTests.cs index 3942216dc24..b78a20e3d7c 100644 --- a/test/standalone/Garnet.test.scripting/LuaScriptTests.cs +++ b/test/standalone/Garnet.test.scripting/LuaScriptTests.cs @@ -1503,13 +1503,14 @@ public void Issue1079() public void ScriptInputsRejectPrecompiledLuaBytecode() { const string Key = "binary-chunk-key"; - const string Script = "return string.dump(function() return redis.call('SET', KEYS[1], 'binary-chunk-executed') end, true)"; using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(); // Generate valid bytecode matching the server's exact Lua version. - var binaryChunk = (byte[])db.ScriptEvaluate(Script); + ClassicAssert.IsTrue(LuaRunner.TryCompileSource("return redis.call('SET', KEYS[1], 'binary-chunk-executed')"u8, out var compiledScript, out var compileError)); + ClassicAssert.IsNull(compileError); + var binaryChunk = compiledScript.Data.ToArray(); ClassicAssert.GreaterOrEqual(binaryChunk.Length, 4); CollectionAssert.AreEqual(new byte[] { 0x1B, (byte)'L', (byte)'u', (byte)'a' }, binaryChunk.AsSpan(0, 4).ToArray()); From adfe785db7eb725f452262ab1cf7aff844d6983a Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Tue, 15 Sep 2026 15:36:51 -0700 Subject: [PATCH 06/27] Assert rejected Lua bytecode has no side effect Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- test/standalone/Garnet.test.scripting/LuaScriptTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/standalone/Garnet.test.scripting/LuaScriptTests.cs b/test/standalone/Garnet.test.scripting/LuaScriptTests.cs index b78a20e3d7c..eec40dc8f2c 100644 --- a/test/standalone/Garnet.test.scripting/LuaScriptTests.cs +++ b/test/standalone/Garnet.test.scripting/LuaScriptTests.cs @@ -1518,6 +1518,7 @@ public void ScriptInputsRejectPrecompiledLuaBytecode() var exception = ClassicAssert.Throws(() => db.Execute("EVAL", [binaryChunk, 1, Key])); StringAssert.Contains("binary chunk", exception.Message); ClassicAssert.IsFalse(db.KeyExists(Key)); + ClassicAssert.AreNotEqual("binary-chunk-executed", (string)db.StringGet(Key)); // SCRIPT LOAD must reject the same bytes without adding them to the global cache. var hash = Convert.ToHexString(SHA1.HashData(binaryChunk)).ToLowerInvariant(); From ae90c7f18cf4290a7c5813078277a6d253e301ea Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Tue, 15 Sep 2026 15:53:52 -0700 Subject: [PATCH 07/27] Centralize persistent script hash allocation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- libs/server/Lua/LuaCommands.cs | 8 ++------ libs/server/Lua/ScriptHashKey.cs | 16 ++++++++++++++++ libs/server/Lua/SessionScriptCache.cs | 5 +---- .../Garnet.test.scripting/LuaScriptTests.cs | 5 ++--- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/libs/server/Lua/LuaCommands.cs b/libs/server/Lua/LuaCommands.cs index bb3106454ea..2196efd238b 100644 --- a/libs/server/Lua/LuaCommands.cs +++ b/libs/server/Lua/LuaCommands.cs @@ -130,9 +130,7 @@ private unsafe bool TryEVAL() // This may strike you as odd, but it is how Redis behaves if (digestOnHeap == null) { - var newAlloc = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); - digest.CopyTo(newAlloc); - if (!storeWrapper.storeScriptCache.TryAdd(new(newAlloc), sessionScriptHandle)) + if (!storeWrapper.storeScriptCache.TryAdd(ScriptHashKey.CopyFrom(digest), sessionScriptHandle)) { // Some other session loaded the script, toss our new handle // @@ -287,9 +285,7 @@ private bool NetworkScriptLoad() { if (digestOnHeap == null) { - var newAlloc = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); - digest.CopyTo(newAlloc); - if (!storeWrapper.storeScriptCache.TryAdd(new(newAlloc), sessionScriptHandle)) + if (!storeWrapper.storeScriptCache.TryAdd(ScriptHashKey.CopyFrom(digest), sessionScriptHandle)) { // Some other caller added the script already, our new handle is dead // but we'll load it from the shared cache on next invocation diff --git a/libs/server/Lua/ScriptHashKey.cs b/libs/server/Lua/ScriptHashKey.cs index 6908029993a..7643103fdbf 100644 --- a/libs/server/Lua/ScriptHashKey.cs +++ b/libs/server/Lua/ScriptHashKey.cs @@ -32,6 +32,22 @@ internal unsafe ScriptHashKey(byte[] pohArr) arrRef = pohArr; } + internal static ScriptHashKey CopyFrom(ReadOnlySpan hash) + { + Debug.Assert(hash.Length == SessionScriptCache.SHA1Len, "Only one valid length for script hash keys"); + + var pinnedHash = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); + hash.CopyTo(pinnedHash); + return new(pinnedHash); + } + + internal static ScriptHashKey CopyFrom(ScriptHashKey hash) + { + var pinnedHash = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); + hash.CopyTo(pinnedHash); + return new(pinnedHash); + } + /// /// Copy key data. /// diff --git a/libs/server/Lua/SessionScriptCache.cs b/libs/server/Lua/SessionScriptCache.cs index 82b98b215c4..87176f5045d 100644 --- a/libs/server/Lua/SessionScriptCache.cs +++ b/libs/server/Lua/SessionScriptCache.cs @@ -275,10 +275,7 @@ out ScriptHashKey? digestOnHeap // There's an implicit assumption that all callers are using unmanaged memory. // If that becomes untrue, there's an optimization opportunity to re-use the // managed memory here. - var into = GC.AllocateUninitializedArray(SHA1Len, pinned: true); - digest.CopyTo(into); - - ScriptHashKey storeKeyDigest = new(into); + var storeKeyDigest = ScriptHashKey.CopyFrom(digest); digestOnHeap = storeKeyDigest; luaScriptHandle ??= new(compiledSource); diff --git a/test/standalone/Garnet.test.scripting/LuaScriptTests.cs b/test/standalone/Garnet.test.scripting/LuaScriptTests.cs index eec40dc8f2c..296f4cfcbee 100644 --- a/test/standalone/Garnet.test.scripting/LuaScriptTests.cs +++ b/test/standalone/Garnet.test.scripting/LuaScriptTests.cs @@ -1536,11 +1536,10 @@ public void HostInsertedScriptSourceIsCompiledAsText() var db = redis.GetDatabase(); var source = "return 2"u8; var hash = Convert.ToHexString(SHA1.HashData(source)).ToLowerInvariant(); - var digest = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); - _ = Encoding.ASCII.GetBytes(hash, digest); + var scriptKey = ScriptHashKey.CopyFrom(Encoding.ASCII.GetBytes(hash)); // The public handle constructor accepts source text, not trusted precompiled bytecode. - ClassicAssert.IsTrue(server.Provider.StoreWrapper.storeScriptCache.TryAdd(new ScriptHashKey(digest), new LuaScriptHandle(source.ToArray()))); + ClassicAssert.IsTrue(server.Provider.StoreWrapper.storeScriptCache.TryAdd(scriptKey, new LuaScriptHandle(source.ToArray()))); ClassicAssert.AreEqual(2, (int)db.Execute("EVALSHA", hash, 0)); } From 57403d870f53a1d286fda47d1164655fd25963c0 Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Tue, 15 Sep 2026 16:03:22 -0700 Subject: [PATCH 08/27] Remove internal script cache test Keep regression coverage focused on supported network-facing scripting workflows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- .../Garnet.test.scripting/LuaScriptTests.cs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/test/standalone/Garnet.test.scripting/LuaScriptTests.cs b/test/standalone/Garnet.test.scripting/LuaScriptTests.cs index 296f4cfcbee..3914e18ffae 100644 --- a/test/standalone/Garnet.test.scripting/LuaScriptTests.cs +++ b/test/standalone/Garnet.test.scripting/LuaScriptTests.cs @@ -1529,20 +1529,6 @@ public void ScriptInputsRejectPrecompiledLuaBytecode() ClassicAssert.AreEqual(0, (int)exists[0]); } - [Test] - public void HostInsertedScriptSourceIsCompiledAsText() - { - using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); - var db = redis.GetDatabase(); - var source = "return 2"u8; - var hash = Convert.ToHexString(SHA1.HashData(source)).ToLowerInvariant(); - var scriptKey = ScriptHashKey.CopyFrom(Encoding.ASCII.GetBytes(hash)); - - // The public handle constructor accepts source text, not trusted precompiled bytecode. - ClassicAssert.IsTrue(server.Provider.StoreWrapper.storeScriptCache.TryAdd(scriptKey, new LuaScriptHandle(source.ToArray()))); - ClassicAssert.AreEqual(2, (int)db.Execute("EVALSHA", hash, 0)); - } - [Test] public void EvalUsesFullSourceLength() { From 5e15493fd777ef6471dcc17b7c2cc7f70c043b89 Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Tue, 15 Sep 2026 16:07:58 -0700 Subject: [PATCH 09/27] Keep script hash handling unchanged Remove the unrelated ScriptHashKey allocation refactor from the Lua bytecode fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- libs/server/Lua/LuaCommands.cs | 8 ++++++-- libs/server/Lua/ScriptHashKey.cs | 16 ---------------- libs/server/Lua/SessionScriptCache.cs | 5 ++++- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/libs/server/Lua/LuaCommands.cs b/libs/server/Lua/LuaCommands.cs index 2196efd238b..bb3106454ea 100644 --- a/libs/server/Lua/LuaCommands.cs +++ b/libs/server/Lua/LuaCommands.cs @@ -130,7 +130,9 @@ private unsafe bool TryEVAL() // This may strike you as odd, but it is how Redis behaves if (digestOnHeap == null) { - if (!storeWrapper.storeScriptCache.TryAdd(ScriptHashKey.CopyFrom(digest), sessionScriptHandle)) + var newAlloc = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); + digest.CopyTo(newAlloc); + if (!storeWrapper.storeScriptCache.TryAdd(new(newAlloc), sessionScriptHandle)) { // Some other session loaded the script, toss our new handle // @@ -285,7 +287,9 @@ private bool NetworkScriptLoad() { if (digestOnHeap == null) { - if (!storeWrapper.storeScriptCache.TryAdd(ScriptHashKey.CopyFrom(digest), sessionScriptHandle)) + var newAlloc = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); + digest.CopyTo(newAlloc); + if (!storeWrapper.storeScriptCache.TryAdd(new(newAlloc), sessionScriptHandle)) { // Some other caller added the script already, our new handle is dead // but we'll load it from the shared cache on next invocation diff --git a/libs/server/Lua/ScriptHashKey.cs b/libs/server/Lua/ScriptHashKey.cs index 7643103fdbf..6908029993a 100644 --- a/libs/server/Lua/ScriptHashKey.cs +++ b/libs/server/Lua/ScriptHashKey.cs @@ -32,22 +32,6 @@ internal unsafe ScriptHashKey(byte[] pohArr) arrRef = pohArr; } - internal static ScriptHashKey CopyFrom(ReadOnlySpan hash) - { - Debug.Assert(hash.Length == SessionScriptCache.SHA1Len, "Only one valid length for script hash keys"); - - var pinnedHash = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); - hash.CopyTo(pinnedHash); - return new(pinnedHash); - } - - internal static ScriptHashKey CopyFrom(ScriptHashKey hash) - { - var pinnedHash = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); - hash.CopyTo(pinnedHash); - return new(pinnedHash); - } - /// /// Copy key data. /// diff --git a/libs/server/Lua/SessionScriptCache.cs b/libs/server/Lua/SessionScriptCache.cs index 87176f5045d..82b98b215c4 100644 --- a/libs/server/Lua/SessionScriptCache.cs +++ b/libs/server/Lua/SessionScriptCache.cs @@ -275,7 +275,10 @@ out ScriptHashKey? digestOnHeap // There's an implicit assumption that all callers are using unmanaged memory. // If that becomes untrue, there's an optimization opportunity to re-use the // managed memory here. - var storeKeyDigest = ScriptHashKey.CopyFrom(digest); + var into = GC.AllocateUninitializedArray(SHA1Len, pinned: true); + digest.CopyTo(into); + + ScriptHashKey storeKeyDigest = new(into); digestOnHeap = storeKeyDigest; luaScriptHandle ??= new(compiledSource); From baefea588f272f80dac574c27c2310336e23c241 Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 10:22:04 -0700 Subject: [PATCH 10/27] Make Lua source handle creation explicit Replace the implicit source-data constructor with a named FromSource factory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- .../BDN.benchmark/Lua/LuaScriptCacheOperations.cs | 4 ++-- libs/server/Lua/LuaScriptHandle.cs | 11 +++-------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs b/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs index de4f382a65a..f613bde679e 100644 --- a/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs +++ b/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs @@ -51,14 +51,14 @@ public void GlobalSetup() outerHitDigest = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); sessionScriptCache.GetScriptDigest("return 1"u8, outerHitDigest); - if (!storeWrapper.storeScriptCache.TryAdd(new(outerHitDigest), new("return 1"u8.ToArray()))) + if (!storeWrapper.storeScriptCache.TryAdd(new(outerHitDigest), LuaScriptHandle.FromSource("return 1"u8.ToArray()))) { throw new InvalidOperationException("Should have been able to load into global cache"); } innerHitDigest = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); sessionScriptCache.GetScriptDigest("return 1 + 1"u8, innerHitDigest); - if (!storeWrapper.storeScriptCache.TryAdd(new(innerHitDigest), new("return 1 + 1"u8.ToArray()))) + if (!storeWrapper.storeScriptCache.TryAdd(new(innerHitDigest), LuaScriptHandle.FromSource("return 1 + 1"u8.ToArray()))) { throw new InvalidOperationException("Should have been able to load into global cache"); } diff --git a/libs/server/Lua/LuaScriptHandle.cs b/libs/server/Lua/LuaScriptHandle.cs index 196ac66817b..04d4694f701 100644 --- a/libs/server/Lua/LuaScriptHandle.cs +++ b/libs/server/Lua/LuaScriptHandle.cs @@ -37,15 +37,10 @@ public sealed class LuaScriptHandle : IDisposable /// Creates a handle for Lua source text. /// /// Lua source text. - public LuaScriptHandle(ReadOnlyMemory scriptData) - : this(new LuaScriptChunk(scriptData, LuaScriptChunkKind.Text)) - { - } + /// A handle containing Lua source text. + public static LuaScriptHandle FromSource(ReadOnlyMemory scriptData) => new(new LuaScriptChunk(scriptData, LuaScriptChunkKind.Text)); - internal LuaScriptHandle(LuaScriptChunk chunk) - { - Chunk = chunk; - } + internal LuaScriptHandle(LuaScriptChunk chunk) => Chunk = chunk; /// public void Dispose() From 9120748d16e1fb03ac4f96f1691474d6c641d4d9 Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 10:37:47 -0700 Subject: [PATCH 11/27] Simplify Lua script cache branches Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- libs/server/Lua/LuaRunner.cs | 26 ++--------------- libs/server/Lua/SessionScriptCache.cs | 40 ++++++--------------------- 2 files changed, 11 insertions(+), 55 deletions(-) diff --git a/libs/server/Lua/LuaRunner.cs b/libs/server/Lua/LuaRunner.cs index 10ec29db641..7e43ae9b035 100644 --- a/libs/server/Lua/LuaRunner.cs +++ b/libs/server/Lua/LuaRunner.cs @@ -203,34 +203,12 @@ public bool NeedsDispose /// /// Creates a new runner with the source of the script /// - public unsafe LuaRunner( - LuaMemoryManagementMode memMode, - int? memLimitBytes, - LuaLoggingMode logMode, - HashSet allowedFunctions, - ReadOnlyMemory source, - bool txnMode = false, - RespServerSession respServerSession = null, - ScratchBufferNetworkSender scratchBufferNetworkSender = null, - string redisVersion = "0.0.0.0", - ILogger logger = null - ) + public unsafe LuaRunner(LuaMemoryManagementMode memMode, int? memLimitBytes, LuaLoggingMode logMode, HashSet allowedFunctions, ReadOnlyMemory source, bool txnMode = false, RespServerSession respServerSession = null, ScratchBufferNetworkSender scratchBufferNetworkSender = null, string redisVersion = "0.0.0.0", ILogger logger = null) : this(memMode, memLimitBytes, logMode, allowedFunctions, new LuaScriptChunk(source, LuaScriptChunkKind.Text), txnMode, respServerSession, scratchBufferNetworkSender, redisVersion, logger) { } - internal unsafe LuaRunner( - LuaMemoryManagementMode memMode, - int? memLimitBytes, - LuaLoggingMode logMode, - HashSet allowedFunctions, - LuaScriptChunk source, - bool txnMode = false, - RespServerSession respServerSession = null, - ScratchBufferNetworkSender scratchBufferNetworkSender = null, - string redisVersion = "0.0.0.0", - ILogger logger = null - ) + internal unsafe LuaRunner(LuaMemoryManagementMode memMode, int? memLimitBytes, LuaLoggingMode logMode, HashSet allowedFunctions, LuaScriptChunk source, bool txnMode = false, RespServerSession respServerSession = null, ScratchBufferNetworkSender scratchBufferNetworkSender = null, string redisVersion = "0.0.0.0", ILogger logger = null) { // KEYS and ARGV are always access by index, and to avoid allocation concerns // we also want to track their 'array'-bits sizes diff --git a/libs/server/Lua/SessionScriptCache.cs b/libs/server/Lua/SessionScriptCache.cs index 82b98b215c4..8edb48f2368 100644 --- a/libs/server/Lua/SessionScriptCache.cs +++ b/libs/server/Lua/SessionScriptCache.cs @@ -183,22 +183,15 @@ out ScriptHashKey? digestOnHeap return true; } - if (luaScriptHandle != null) - return TryLoadCached(session, digest, ref luaScriptHandle, out runner, out digestOnHeap); - - return TryCompileAndLoad(session, source, digest, ref luaScriptHandle, out runner, out digestOnHeap); + return luaScriptHandle != null + ? TryLoadCached(session, digest, ref luaScriptHandle, out runner, out digestOnHeap) + : TryCompileAndLoad(session, source, digest, ref luaScriptHandle, out runner, out digestOnHeap); } /// /// Load a script previously stored in the global cache. /// - internal bool TryLoadCached( - RespServerSession session, - ScriptHashKey digest, - ref LuaScriptHandle luaScriptHandle, - out LuaRunner runner, - out ScriptHashKey? digestOnHeap - ) + internal bool TryLoadCached(RespServerSession session, ScriptHashKey digest, ref LuaScriptHandle luaScriptHandle, out LuaRunner runner, out ScriptHashKey? digestOnHeap) { if (TryGetFromDigest(digest, out runner, out var existingLuaScriptHandle)) { @@ -207,20 +200,12 @@ out ScriptHashKey? digestOnHeap return true; } - if (luaScriptHandle.Chunk.Kind == LuaScriptChunkKind.GarnetGeneratedBinary) - return TryLoadCompiled(session, luaScriptHandle.Chunk, digest, ref luaScriptHandle, out runner, out digestOnHeap); - - return TryCompileAndLoad(session, luaScriptHandle.ScriptData.Span, digest, ref luaScriptHandle, out runner, out digestOnHeap); + return luaScriptHandle.Chunk.Kind == LuaScriptChunkKind.GarnetGeneratedBinary + ? TryLoadCompiled(session, luaScriptHandle.Chunk, digest, ref luaScriptHandle, out runner, out digestOnHeap) + : TryCompileAndLoad(session, luaScriptHandle.ScriptData.Span, digest, ref luaScriptHandle, out runner, out digestOnHeap); } - private bool TryCompileAndLoad( - RespServerSession session, - ReadOnlySpan source, - ScriptHashKey digest, - ref LuaScriptHandle luaScriptHandle, - out LuaRunner runner, - out ScriptHashKey? digestOnHeap - ) + private bool TryCompileAndLoad(RespServerSession session, ReadOnlySpan source, ScriptHashKey digest, ref LuaScriptHandle luaScriptHandle, out LuaRunner runner, out ScriptHashKey? digestOnHeap) { LuaScriptChunk compiledSource; string error; @@ -247,14 +232,7 @@ out ScriptHashKey? digestOnHeap /// /// Load internally compiled script bytecode into the cache. /// - private bool TryLoadCompiled( - RespServerSession session, - LuaScriptChunk compiledSource, - ScriptHashKey digest, - ref LuaScriptHandle luaScriptHandle, - out LuaRunner runner, - out ScriptHashKey? digestOnHeap - ) + private bool TryLoadCompiled(RespServerSession session, LuaScriptChunk compiledSource, ScriptHashKey digest, ref LuaScriptHandle luaScriptHandle, out LuaRunner runner, out ScriptHashKey? digestOnHeap) { if (TryGetFromDigest(digest, out runner, out var existingLuaScriptHandle)) { From 8e5b33ff8347ada1a35f3cb0602c188bb0d336fe Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 10:38:43 -0700 Subject: [PATCH 12/27] Restore LuaRunner constructor formatting Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- libs/server/Lua/LuaRunner.cs | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/libs/server/Lua/LuaRunner.cs b/libs/server/Lua/LuaRunner.cs index 7e43ae9b035..10ec29db641 100644 --- a/libs/server/Lua/LuaRunner.cs +++ b/libs/server/Lua/LuaRunner.cs @@ -203,12 +203,34 @@ public bool NeedsDispose /// /// Creates a new runner with the source of the script /// - public unsafe LuaRunner(LuaMemoryManagementMode memMode, int? memLimitBytes, LuaLoggingMode logMode, HashSet allowedFunctions, ReadOnlyMemory source, bool txnMode = false, RespServerSession respServerSession = null, ScratchBufferNetworkSender scratchBufferNetworkSender = null, string redisVersion = "0.0.0.0", ILogger logger = null) + public unsafe LuaRunner( + LuaMemoryManagementMode memMode, + int? memLimitBytes, + LuaLoggingMode logMode, + HashSet allowedFunctions, + ReadOnlyMemory source, + bool txnMode = false, + RespServerSession respServerSession = null, + ScratchBufferNetworkSender scratchBufferNetworkSender = null, + string redisVersion = "0.0.0.0", + ILogger logger = null + ) : this(memMode, memLimitBytes, logMode, allowedFunctions, new LuaScriptChunk(source, LuaScriptChunkKind.Text), txnMode, respServerSession, scratchBufferNetworkSender, redisVersion, logger) { } - internal unsafe LuaRunner(LuaMemoryManagementMode memMode, int? memLimitBytes, LuaLoggingMode logMode, HashSet allowedFunctions, LuaScriptChunk source, bool txnMode = false, RespServerSession respServerSession = null, ScratchBufferNetworkSender scratchBufferNetworkSender = null, string redisVersion = "0.0.0.0", ILogger logger = null) + internal unsafe LuaRunner( + LuaMemoryManagementMode memMode, + int? memLimitBytes, + LuaLoggingMode logMode, + HashSet allowedFunctions, + LuaScriptChunk source, + bool txnMode = false, + RespServerSession respServerSession = null, + ScratchBufferNetworkSender scratchBufferNetworkSender = null, + string redisVersion = "0.0.0.0", + ILogger logger = null + ) { // KEYS and ARGV are always access by index, and to avoid allocation concerns // we also want to track their 'array'-bits sizes From c28562a5415be6772f8e67350cf66ed72dd2808a Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 10:41:19 -0700 Subject: [PATCH 13/27] Finish Lua formatting cleanup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- libs/server/Lua/LuaRunner.cs | 16 ++-------------- libs/server/Lua/LuaStateWrapper.cs | 4 ++++ 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/libs/server/Lua/LuaRunner.cs b/libs/server/Lua/LuaRunner.cs index 10ec29db641..6b182f393db 100644 --- a/libs/server/Lua/LuaRunner.cs +++ b/libs/server/Lua/LuaRunner.cs @@ -214,23 +214,11 @@ public unsafe LuaRunner( ScratchBufferNetworkSender scratchBufferNetworkSender = null, string redisVersion = "0.0.0.0", ILogger logger = null - ) - : this(memMode, memLimitBytes, logMode, allowedFunctions, new LuaScriptChunk(source, LuaScriptChunkKind.Text), txnMode, respServerSession, scratchBufferNetworkSender, redisVersion, logger) + ) : this(memMode, memLimitBytes, logMode, allowedFunctions, new LuaScriptChunk(source, LuaScriptChunkKind.Text), txnMode, respServerSession, scratchBufferNetworkSender, redisVersion, logger) { } - internal unsafe LuaRunner( - LuaMemoryManagementMode memMode, - int? memLimitBytes, - LuaLoggingMode logMode, - HashSet allowedFunctions, - LuaScriptChunk source, - bool txnMode = false, - RespServerSession respServerSession = null, - ScratchBufferNetworkSender scratchBufferNetworkSender = null, - string redisVersion = "0.0.0.0", - ILogger logger = null - ) + internal unsafe LuaRunner(LuaMemoryManagementMode memMode, int? memLimitBytes, LuaLoggingMode logMode, HashSet allowedFunctions, LuaScriptChunk source, bool txnMode = false, RespServerSession respServerSession = null, ScratchBufferNetworkSender scratchBufferNetworkSender = null, string redisVersion = "0.0.0.0", ILogger logger = null) { // KEYS and ARGV are always access by index, and to avoid allocation concerns // we also want to track their 'array'-bits sizes diff --git a/libs/server/Lua/LuaStateWrapper.cs b/libs/server/Lua/LuaStateWrapper.cs index 4ecfa34ed65..29536d1d81b 100644 --- a/libs/server/Lua/LuaStateWrapper.cs +++ b/libs/server/Lua/LuaStateWrapper.cs @@ -463,6 +463,8 @@ internal LuaStatus LoadBinaryBuffer(ReadOnlySpan buffer) { AssertLuaStackNotFull(2); + // luaL_loadbufferx uses Lua's protected parser and returns failures as status values with an error on the stack. + // See https://www.lua.org/source/5.4/lauxlib.c.html#luaL_loadbufferx. var ret = NativeMethods.LoadBinaryBuffer(state, buffer); if (ret != LuaStatus.OK) @@ -489,6 +491,8 @@ internal LuaStatus LoadTextBuffer(ReadOnlySpan buffer) AssertLuaStackNotFull(2); // Text-only mode rejects binary chunks at the untrusted input boundary. + // luaL_loadbufferx uses Lua's protected parser and returns failures as status values with an error on the stack. + // See https://www.lua.org/source/5.4/lauxlib.c.html#luaL_loadbufferx. var ret = NativeMethods.LoadTextBuffer(state, buffer); if (ret != LuaStatus.OK) From 4e7c1d25a62a424233f3baf33e44877e32e84436 Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 11:02:45 -0700 Subject: [PATCH 14/27] Clarify Lua source factory name Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs | 4 ++-- libs/server/Lua/LuaScriptHandle.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs b/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs index f613bde679e..d4dd64a4d9a 100644 --- a/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs +++ b/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs @@ -51,14 +51,14 @@ public void GlobalSetup() outerHitDigest = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); sessionScriptCache.GetScriptDigest("return 1"u8, outerHitDigest); - if (!storeWrapper.storeScriptCache.TryAdd(new(outerHitDigest), LuaScriptHandle.FromSource("return 1"u8.ToArray()))) + if (!storeWrapper.storeScriptCache.TryAdd(new(outerHitDigest), LuaScriptHandle.FromTextSource("return 1"u8.ToArray()))) { throw new InvalidOperationException("Should have been able to load into global cache"); } innerHitDigest = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); sessionScriptCache.GetScriptDigest("return 1 + 1"u8, innerHitDigest); - if (!storeWrapper.storeScriptCache.TryAdd(new(innerHitDigest), LuaScriptHandle.FromSource("return 1 + 1"u8.ToArray()))) + if (!storeWrapper.storeScriptCache.TryAdd(new(innerHitDigest), LuaScriptHandle.FromTextSource("return 1 + 1"u8.ToArray()))) { throw new InvalidOperationException("Should have been able to load into global cache"); } diff --git a/libs/server/Lua/LuaScriptHandle.cs b/libs/server/Lua/LuaScriptHandle.cs index 04d4694f701..adcc165a587 100644 --- a/libs/server/Lua/LuaScriptHandle.cs +++ b/libs/server/Lua/LuaScriptHandle.cs @@ -38,7 +38,7 @@ public sealed class LuaScriptHandle : IDisposable /// /// Lua source text. /// A handle containing Lua source text. - public static LuaScriptHandle FromSource(ReadOnlyMemory scriptData) => new(new LuaScriptChunk(scriptData, LuaScriptChunkKind.Text)); + public static LuaScriptHandle FromTextSource(ReadOnlyMemory scriptData) => new(new LuaScriptChunk(scriptData, LuaScriptChunkKind.Text)); internal LuaScriptHandle(LuaScriptChunk chunk) => Chunk = chunk; From 49cbfbc498c0e27fd416777856ddf749b326424a Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 11:03:04 -0700 Subject: [PATCH 15/27] Remove redundant Lua loader comments Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- libs/server/Lua/LuaStateWrapper.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/libs/server/Lua/LuaStateWrapper.cs b/libs/server/Lua/LuaStateWrapper.cs index 29536d1d81b..4ecfa34ed65 100644 --- a/libs/server/Lua/LuaStateWrapper.cs +++ b/libs/server/Lua/LuaStateWrapper.cs @@ -463,8 +463,6 @@ internal LuaStatus LoadBinaryBuffer(ReadOnlySpan buffer) { AssertLuaStackNotFull(2); - // luaL_loadbufferx uses Lua's protected parser and returns failures as status values with an error on the stack. - // See https://www.lua.org/source/5.4/lauxlib.c.html#luaL_loadbufferx. var ret = NativeMethods.LoadBinaryBuffer(state, buffer); if (ret != LuaStatus.OK) @@ -491,8 +489,6 @@ internal LuaStatus LoadTextBuffer(ReadOnlySpan buffer) AssertLuaStackNotFull(2); // Text-only mode rejects binary chunks at the untrusted input boundary. - // luaL_loadbufferx uses Lua's protected parser and returns failures as status values with an error on the stack. - // See https://www.lua.org/source/5.4/lauxlib.c.html#luaL_loadbufferx. var ret = NativeMethods.LoadTextBuffer(state, buffer); if (ret != LuaStatus.OK) From 1291fa85e80948daa051f8fd4ecbe0a9be8475f7 Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 11:07:24 -0700 Subject: [PATCH 16/27] Clarify Lua script cache control flow Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- libs/server/Lua/SessionScriptCache.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libs/server/Lua/SessionScriptCache.cs b/libs/server/Lua/SessionScriptCache.cs index 8edb48f2368..46742190c14 100644 --- a/libs/server/Lua/SessionScriptCache.cs +++ b/libs/server/Lua/SessionScriptCache.cs @@ -183,9 +183,10 @@ out ScriptHashKey? digestOnHeap return true; } - return luaScriptHandle != null - ? TryLoadCached(session, digest, ref luaScriptHandle, out runner, out digestOnHeap) - : TryCompileAndLoad(session, source, digest, ref luaScriptHandle, out runner, out digestOnHeap); + if (luaScriptHandle != null) + return TryLoadCached(session, digest, ref luaScriptHandle, out runner, out digestOnHeap); + + return TryCompileAndLoad(session, source, digest, ref luaScriptHandle, out runner, out digestOnHeap); } /// From 15efd15522513e053e21c8103b04664c10c81ce9 Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 11:12:55 -0700 Subject: [PATCH 17/27] Separate Lua source and cache loading Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- .../Lua/LuaScriptCacheOperations.cs | 5 +-- libs/server/Lua/LuaCommands.cs | 38 ++++++++++++++----- libs/server/Lua/SessionScriptCache.cs | 22 ++++------- 3 files changed, 39 insertions(+), 26 deletions(-) diff --git a/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs b/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs index d4dd64a4d9a..3df4fb36b7a 100644 --- a/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs +++ b/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs @@ -81,8 +81,7 @@ public void IterationSetup() sessionScriptCache.Clear(); // Make outer hit available for every iteration - LuaScriptHandle scriptHandle = null; - if (!sessionScriptCache.TryLoadSource(session, "return 1"u8, new(outerHitDigest), ref scriptHandle, out _, out _)) + if (!sessionScriptCache.TryLoadSource(session, "return 1"u8, new(outerHitDigest), out _, out _, out _)) { throw new InvalidOperationException("Should have been able to load"); } @@ -147,7 +146,7 @@ private void LoadScript(Span digest) { if (storeWrapper.storeScriptCache.TryGetValue(digestKey, out var scriptHandle)) { - if (!sessionScriptCache.TryLoadCached(session, digestKey, ref scriptHandle, out runner, out _)) + if (!sessionScriptCache.TryLoadCached(session, digestKey, scriptHandle, out runner)) { // TryLoadCached will have written an error out, if any diff --git a/libs/server/Lua/LuaCommands.cs b/libs/server/Lua/LuaCommands.cs index bb3106454ea..aa2c206fc15 100644 --- a/libs/server/Lua/LuaCommands.cs +++ b/libs/server/Lua/LuaCommands.cs @@ -48,7 +48,7 @@ private unsafe bool TryEVALSHA() { if (storeWrapper.storeScriptCache.TryGetValue(scriptKey, out var globalScriptHandle)) { - if (!sessionScriptCache.TryLoadCached(this, scriptKey, ref globalScriptHandle, out runner, out _)) + if (!sessionScriptCache.TryLoadCached(this, scriptKey, globalScriptHandle, out runner)) { // TryLoadCached will have written an error out, if any // @@ -116,14 +116,23 @@ private unsafe bool TryEVAL() var onStackScriptKey = new ScriptHashKey(digest); _ = storeWrapper.storeScriptCache.TryGetValue(onStackScriptKey, out var globalScriptHandle); - var sessionScriptHandle = globalScriptHandle; - - if (!sessionScriptCache.TryLoadSource(this, script.ReadOnlySpan, onStackScriptKey, ref sessionScriptHandle, out var runner, out var digestOnHeap)) + LuaRunner runner; + LuaScriptHandle sessionScriptHandle; + ScriptHashKey? digestOnHeap; + if (globalScriptHandle != null) + { + sessionScriptHandle = globalScriptHandle; + digestOnHeap = null; + if (!sessionScriptCache.TryLoadCached(this, onStackScriptKey, globalScriptHandle, out runner)) + return true; + } + else if (!sessionScriptCache.TryLoadSource(this, script.ReadOnlySpan, onStackScriptKey, out sessionScriptHandle, out runner, out digestOnHeap)) { // TryLoadSource will have written any errors out return true; } - else if (sessionScriptHandle != globalScriptHandle) + + if (globalScriptHandle == null) { // Add script to the store dictionary IF we didn't already have it cached // @@ -277,13 +286,24 @@ private bool NetworkScriptLoad() var onStackScriptHashKey = new ScriptHashKey(digest); _ = storeWrapper.storeScriptCache.TryGetValue(onStackScriptHashKey, out var globalScriptHandle); - var sessionScriptHandle = globalScriptHandle; - if (sessionScriptCache.TryLoadSource(this, source.ReadOnlySpan, onStackScriptHashKey, ref sessionScriptHandle, out _, out var digestOnHeap)) + LuaScriptHandle sessionScriptHandle; + ScriptHashKey? digestOnHeap; + var loaded = false; + if (globalScriptHandle != null) { - // TryLoadSource will write any errors out + sessionScriptHandle = globalScriptHandle; + digestOnHeap = null; + loaded = sessionScriptCache.TryLoadCached(this, onStackScriptHashKey, globalScriptHandle, out _); + } + else + { + loaded = sessionScriptCache.TryLoadSource(this, source.ReadOnlySpan, onStackScriptHashKey, out sessionScriptHandle, out _, out digestOnHeap); + } + if (loaded) + { // Add script to the global store dictionary if not already in there - if (globalScriptHandle != sessionScriptHandle) + if (globalScriptHandle == null) { if (digestOnHeap == null) { diff --git a/libs/server/Lua/SessionScriptCache.cs b/libs/server/Lua/SessionScriptCache.cs index 46742190c14..b676c793293 100644 --- a/libs/server/Lua/SessionScriptCache.cs +++ b/libs/server/Lua/SessionScriptCache.cs @@ -163,7 +163,7 @@ public bool TryGetFromDigest(ScriptHashKey digest, out LuaRunner scriptRunner, o } /// - /// Load script into the cache. + /// Compile Lua source text and load it into the session cache. /// /// If necessary, will be set so the allocation can be reused. /// @@ -171,7 +171,7 @@ internal bool TryLoadSource( RespServerSession session, ReadOnlySpan source, ScriptHashKey digest, - ref LuaScriptHandle luaScriptHandle, + out LuaScriptHandle luaScriptHandle, out LuaRunner runner, out ScriptHashKey? digestOnHeap ) @@ -183,27 +183,21 @@ out ScriptHashKey? digestOnHeap return true; } - if (luaScriptHandle != null) - return TryLoadCached(session, digest, ref luaScriptHandle, out runner, out digestOnHeap); - + luaScriptHandle = null; return TryCompileAndLoad(session, source, digest, ref luaScriptHandle, out runner, out digestOnHeap); } /// /// Load a script previously stored in the global cache. /// - internal bool TryLoadCached(RespServerSession session, ScriptHashKey digest, ref LuaScriptHandle luaScriptHandle, out LuaRunner runner, out ScriptHashKey? digestOnHeap) + internal bool TryLoadCached(RespServerSession session, ScriptHashKey digest, LuaScriptHandle cachedScriptHandle, out LuaRunner runner) { - if (TryGetFromDigest(digest, out runner, out var existingLuaScriptHandle)) - { - luaScriptHandle = existingLuaScriptHandle; - digestOnHeap = null; + if (TryGetFromDigest(digest, out runner, out _)) return true; - } - return luaScriptHandle.Chunk.Kind == LuaScriptChunkKind.GarnetGeneratedBinary - ? TryLoadCompiled(session, luaScriptHandle.Chunk, digest, ref luaScriptHandle, out runner, out digestOnHeap) - : TryCompileAndLoad(session, luaScriptHandle.ScriptData.Span, digest, ref luaScriptHandle, out runner, out digestOnHeap); + return cachedScriptHandle.Chunk.Kind == LuaScriptChunkKind.GarnetGeneratedBinary + ? TryLoadCompiled(session, cachedScriptHandle.Chunk, digest, ref cachedScriptHandle, out runner, out _) + : TryCompileAndLoad(session, cachedScriptHandle.ScriptData.Span, digest, ref cachedScriptHandle, out runner, out _); } private bool TryCompileAndLoad(RespServerSession session, ReadOnlySpan source, ScriptHashKey digest, ref LuaScriptHandle luaScriptHandle, out LuaRunner runner, out ScriptHashKey? digestOnHeap) From fa64aafab7bf11eb5141f91d2e213388d59971cd Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 11:20:36 -0700 Subject: [PATCH 18/27] Centralize Lua script loading Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- libs/server/Lua/LuaCommands.cs | 109 ++++++++------------------------- 1 file changed, 25 insertions(+), 84 deletions(-) diff --git a/libs/server/Lua/LuaCommands.cs b/libs/server/Lua/LuaCommands.cs index aa2c206fc15..b55cc2f2641 100644 --- a/libs/server/Lua/LuaCommands.cs +++ b/libs/server/Lua/LuaCommands.cs @@ -114,52 +114,8 @@ private unsafe bool TryEVAL() sessionScriptCache.GetScriptDigest(script.ReadOnlySpan, digest); var onStackScriptKey = new ScriptHashKey(digest); - _ = storeWrapper.storeScriptCache.TryGetValue(onStackScriptKey, out var globalScriptHandle); - - LuaRunner runner; - LuaScriptHandle sessionScriptHandle; - ScriptHashKey? digestOnHeap; - if (globalScriptHandle != null) - { - sessionScriptHandle = globalScriptHandle; - digestOnHeap = null; - if (!sessionScriptCache.TryLoadCached(this, onStackScriptKey, globalScriptHandle, out runner)) - return true; - } - else if (!sessionScriptCache.TryLoadSource(this, script.ReadOnlySpan, onStackScriptKey, out sessionScriptHandle, out runner, out digestOnHeap)) - { - // TryLoadSource will have written any errors out + if (!TryLoadScriptForSession(script.ReadOnlySpan, digest, onStackScriptKey, out var runner)) return true; - } - - if (globalScriptHandle == null) - { - // Add script to the store dictionary IF we didn't already have it cached - // - // This may strike you as odd, but it is how Redis behaves - if (digestOnHeap == null) - { - var newAlloc = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); - digest.CopyTo(newAlloc); - if (!storeWrapper.storeScriptCache.TryAdd(new(newAlloc), sessionScriptHandle)) - { - // Some other session loaded the script, toss our new handle - // - // Next time this script is run, it'll be pulled from the global cache - sessionScriptHandle.Dispose(); - } - } - else - { - if (!storeWrapper.storeScriptCache.TryAdd(digestOnHeap.Value, sessionScriptHandle)) - { - // Some other session loaded the script, toss our new handle - // - // Next time this script is run, it'll be pulled from the global cache - sessionScriptHandle.Dispose(); - } - } - } if (runner == null) { @@ -284,52 +240,37 @@ private bool NetworkScriptLoad() sessionScriptCache.GetScriptDigest(source.Span, digest); var onStackScriptHashKey = new ScriptHashKey(digest); - _ = storeWrapper.storeScriptCache.TryGetValue(onStackScriptHashKey, out var globalScriptHandle); + if (TryLoadScriptForSession(source.ReadOnlySpan, digest, onStackScriptHashKey, out _)) + { + while (!RespWriteUtils.TryWriteBulkString(digest, ref dcurr, dend)) + SendAndReset(); + } - LuaScriptHandle sessionScriptHandle; - ScriptHashKey? digestOnHeap; - var loaded = false; - if (globalScriptHandle != null) + return true; + } + + private bool TryLoadScriptForSession(ReadOnlySpan source, ReadOnlySpan digest, ScriptHashKey scriptKey, out LuaRunner runner) + { + if (storeWrapper.storeScriptCache.TryGetValue(scriptKey, out var globalScriptHandle)) + return sessionScriptCache.TryLoadCached(this, scriptKey, globalScriptHandle, out runner); + + if (!sessionScriptCache.TryLoadSource(this, source, scriptKey, out var sessionScriptHandle, out runner, out var digestOnHeap)) + return false; + + ScriptHashKey globalScriptKey; + if (digestOnHeap != null) { - sessionScriptHandle = globalScriptHandle; - digestOnHeap = null; - loaded = sessionScriptCache.TryLoadCached(this, onStackScriptHashKey, globalScriptHandle, out _); + globalScriptKey = digestOnHeap.Value; } else { - loaded = sessionScriptCache.TryLoadSource(this, source.ReadOnlySpan, onStackScriptHashKey, out sessionScriptHandle, out _, out digestOnHeap); + var digestCopy = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); + digest.CopyTo(digestCopy); + globalScriptKey = new(digestCopy); } - if (loaded) - { - // Add script to the global store dictionary if not already in there - if (globalScriptHandle == null) - { - if (digestOnHeap == null) - { - var newAlloc = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); - digest.CopyTo(newAlloc); - if (!storeWrapper.storeScriptCache.TryAdd(new(newAlloc), sessionScriptHandle)) - { - // Some other caller added the script already, our new handle is dead - // but we'll load it from the shared cache on next invocation - sessionScriptHandle.Dispose(); - } - } - else - { - if (!storeWrapper.storeScriptCache.TryAdd(digestOnHeap.Value, sessionScriptHandle)) - { - // Some other caller added the script already, our new handle is dead - // but we'll load it from the shared cache on next invocation - sessionScriptHandle.Dispose(); - } - } - } - - while (!RespWriteUtils.TryWriteBulkString(digest, ref dcurr, dend)) - SendAndReset(); - } + if (!storeWrapper.storeScriptCache.TryAdd(globalScriptKey, sessionScriptHandle)) + sessionScriptHandle.Dispose(); return true; } From 9bde2d909e4fd005d5d9e24cf41dc443ffabffaa Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 11:28:50 -0700 Subject: [PATCH 19/27] Simplify LuaRunner chunk construction Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- libs/server/Lua/LuaRunner.cs | 10 +++------- test/Garnet.fuzz/Targets/LuaScriptCompilation.cs | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/libs/server/Lua/LuaRunner.cs b/libs/server/Lua/LuaRunner.cs index 6b182f393db..6a60ec09fe4 100644 --- a/libs/server/Lua/LuaRunner.cs +++ b/libs/server/Lua/LuaRunner.cs @@ -208,17 +208,13 @@ public unsafe LuaRunner( int? memLimitBytes, LuaLoggingMode logMode, HashSet allowedFunctions, - ReadOnlyMemory source, + LuaScriptChunk source, bool txnMode = false, RespServerSession respServerSession = null, ScratchBufferNetworkSender scratchBufferNetworkSender = null, string redisVersion = "0.0.0.0", ILogger logger = null - ) : this(memMode, memLimitBytes, logMode, allowedFunctions, new LuaScriptChunk(source, LuaScriptChunkKind.Text), txnMode, respServerSession, scratchBufferNetworkSender, redisVersion, logger) - { - } - - internal unsafe LuaRunner(LuaMemoryManagementMode memMode, int? memLimitBytes, LuaLoggingMode logMode, HashSet allowedFunctions, LuaScriptChunk source, bool txnMode = false, RespServerSession respServerSession = null, ScratchBufferNetworkSender scratchBufferNetworkSender = null, string redisVersion = "0.0.0.0", ILogger logger = null) + ) { // KEYS and ARGV are always access by index, and to avoid allocation concerns // we also want to track their 'array'-bits sizes @@ -413,7 +409,7 @@ static void Register(ref LuaStateWrapper state, ReadOnlySpan name, delegat /// Creates a new runner with the source of the script /// public LuaRunner(LuaOptions options, string source, bool txnMode = false, RespServerSession respServerSession = null, ScratchBufferNetworkSender scratchBufferNetworkSender = null, string redisVersion = "0.0.0.0", ILogger logger = null) - : this(options.MemoryManagementMode, options.GetMemoryLimitBytes(), options.LogMode, options.AllowedFunctions, Encoding.UTF8.GetBytes(source), txnMode, respServerSession, scratchBufferNetworkSender, redisVersion, logger) + : this(options.MemoryManagementMode, options.GetMemoryLimitBytes(), options.LogMode, options.AllowedFunctions, new LuaScriptChunk(Encoding.UTF8.GetBytes(source), LuaScriptChunkKind.Text), txnMode, respServerSession, scratchBufferNetworkSender, redisVersion, logger) { } diff --git a/test/Garnet.fuzz/Targets/LuaScriptCompilation.cs b/test/Garnet.fuzz/Targets/LuaScriptCompilation.cs index 20c2e2052b4..36d8e402a17 100644 --- a/test/Garnet.fuzz/Targets/LuaScriptCompilation.cs +++ b/test/Garnet.fuzz/Targets/LuaScriptCompilation.cs @@ -34,7 +34,7 @@ public static void Fuzz(ReadOnlySpan input) { try { - using var runner = new LuaRunner(op.MemoryManagementMode, op.GetMemoryLimitBytes(), op.LogMode, op.AllowedFunctions, input.ToArray()); + using var runner = new LuaRunner(op.MemoryManagementMode, op.GetMemoryLimitBytes(), op.LogMode, op.AllowedFunctions, new LuaScriptChunk(input.ToArray(), LuaScriptChunkKind.Text)); runner.CompileForRunner(); _ = runner.RunForRunner([], []); From 0d184d1caba0e15c41e749ccf869717da7cddc5d Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 11:41:38 -0700 Subject: [PATCH 20/27] Clarify Lua load failure handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- libs/server/Lua/LuaCommands.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/server/Lua/LuaCommands.cs b/libs/server/Lua/LuaCommands.cs index b55cc2f2641..c7076bfef5d 100644 --- a/libs/server/Lua/LuaCommands.cs +++ b/libs/server/Lua/LuaCommands.cs @@ -115,6 +115,7 @@ private unsafe bool TryEVAL() var onStackScriptKey = new ScriptHashKey(digest); if (!TryLoadScriptForSession(script.ReadOnlySpan, digest, onStackScriptKey, out var runner)) + // The loading error was already written to the response. return true; if (runner == null) From 27ccbd5008d2143f96bc52d3e367cd386690d369 Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 11:42:25 -0700 Subject: [PATCH 21/27] Format Lua load failure comment Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- libs/server/Lua/LuaCommands.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libs/server/Lua/LuaCommands.cs b/libs/server/Lua/LuaCommands.cs index c7076bfef5d..e9948bea426 100644 --- a/libs/server/Lua/LuaCommands.cs +++ b/libs/server/Lua/LuaCommands.cs @@ -115,8 +115,7 @@ private unsafe bool TryEVAL() var onStackScriptKey = new ScriptHashKey(digest); if (!TryLoadScriptForSession(script.ReadOnlySpan, digest, onStackScriptKey, out var runner)) - // The loading error was already written to the response. - return true; + return true; // The loading error was written to the response. if (runner == null) { From a3dd5aa2c7e2077a60aae2ea87fbc7576bc1050a Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 12:01:21 -0700 Subject: [PATCH 22/27] Clarify Lua runner cache naming Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- .../Lua/LuaScriptCacheOperations.cs | 6 ++--- libs/server/Lua/LuaCommands.cs | 14 +++++------ libs/server/Lua/SessionScriptCache.cs | 24 +++++++++---------- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs b/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs index 3df4fb36b7a..be4ec23aea9 100644 --- a/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs +++ b/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs @@ -81,7 +81,7 @@ public void IterationSetup() sessionScriptCache.Clear(); // Make outer hit available for every iteration - if (!sessionScriptCache.TryLoadSource(session, "return 1"u8, new(outerHitDigest), out _, out _, out _)) + if (!sessionScriptCache.TryGetOrCreateRunnerFromSource(session, "return 1"u8, new(outerHitDigest), out _, out _, out _)) { throw new InvalidOperationException("Should have been able to load"); } @@ -146,9 +146,9 @@ private void LoadScript(Span digest) { if (storeWrapper.storeScriptCache.TryGetValue(digestKey, out var scriptHandle)) { - if (!sessionScriptCache.TryLoadCached(session, digestKey, scriptHandle, out runner)) + if (!sessionScriptCache.TryGetOrCreateRunnerFromCachedScript(session, digestKey, scriptHandle, out runner)) { - // TryLoadCached will have written an error out, if any + // The loading error was already written, if any _ = storeWrapper.storeScriptCache.TryRemove(digestKey, out _); } diff --git a/libs/server/Lua/LuaCommands.cs b/libs/server/Lua/LuaCommands.cs index e9948bea426..b60648b3367 100644 --- a/libs/server/Lua/LuaCommands.cs +++ b/libs/server/Lua/LuaCommands.cs @@ -48,9 +48,9 @@ private unsafe bool TryEVALSHA() { if (storeWrapper.storeScriptCache.TryGetValue(scriptKey, out var globalScriptHandle)) { - if (!sessionScriptCache.TryLoadCached(this, scriptKey, globalScriptHandle, out runner)) + if (!sessionScriptCache.TryGetOrCreateRunnerFromCachedScript(this, scriptKey, globalScriptHandle, out runner)) { - // TryLoadCached will have written an error out, if any + // The loading error was already written, if any // // Note we DON'T dispose the script handle because this is just the session cache _ = storeWrapper.storeScriptCache.TryRemove(scriptKey, out _); @@ -114,7 +114,7 @@ private unsafe bool TryEVAL() sessionScriptCache.GetScriptDigest(script.ReadOnlySpan, digest); var onStackScriptKey = new ScriptHashKey(digest); - if (!TryLoadScriptForSession(script.ReadOnlySpan, digest, onStackScriptKey, out var runner)) + if (!TryGetOrCreateScriptRunner(script.ReadOnlySpan, digest, onStackScriptKey, out var runner)) return true; // The loading error was written to the response. if (runner == null) @@ -240,7 +240,7 @@ private bool NetworkScriptLoad() sessionScriptCache.GetScriptDigest(source.Span, digest); var onStackScriptHashKey = new ScriptHashKey(digest); - if (TryLoadScriptForSession(source.ReadOnlySpan, digest, onStackScriptHashKey, out _)) + if (TryGetOrCreateScriptRunner(source.ReadOnlySpan, digest, onStackScriptHashKey, out _)) { while (!RespWriteUtils.TryWriteBulkString(digest, ref dcurr, dend)) SendAndReset(); @@ -249,12 +249,12 @@ private bool NetworkScriptLoad() return true; } - private bool TryLoadScriptForSession(ReadOnlySpan source, ReadOnlySpan digest, ScriptHashKey scriptKey, out LuaRunner runner) + private bool TryGetOrCreateScriptRunner(ReadOnlySpan source, ReadOnlySpan digest, ScriptHashKey scriptKey, out LuaRunner runner) { if (storeWrapper.storeScriptCache.TryGetValue(scriptKey, out var globalScriptHandle)) - return sessionScriptCache.TryLoadCached(this, scriptKey, globalScriptHandle, out runner); + return sessionScriptCache.TryGetOrCreateRunnerFromCachedScript(this, scriptKey, globalScriptHandle, out runner); - if (!sessionScriptCache.TryLoadSource(this, source, scriptKey, out var sessionScriptHandle, out runner, out var digestOnHeap)) + if (!sessionScriptCache.TryGetOrCreateRunnerFromSource(this, source, scriptKey, out var sessionScriptHandle, out runner, out var digestOnHeap)) return false; ScriptHashKey globalScriptKey; diff --git a/libs/server/Lua/SessionScriptCache.cs b/libs/server/Lua/SessionScriptCache.cs index b676c793293..0d9a12cebc6 100644 --- a/libs/server/Lua/SessionScriptCache.cs +++ b/libs/server/Lua/SessionScriptCache.cs @@ -167,7 +167,7 @@ public bool TryGetFromDigest(ScriptHashKey digest, out LuaRunner scriptRunner, o /// /// If necessary, will be set so the allocation can be reused. /// - internal bool TryLoadSource( + internal bool TryGetOrCreateRunnerFromSource( RespServerSession session, ReadOnlySpan source, ScriptHashKey digest, @@ -184,30 +184,30 @@ out ScriptHashKey? digestOnHeap } luaScriptHandle = null; - return TryCompileAndLoad(session, source, digest, ref luaScriptHandle, out runner, out digestOnHeap); + return TryCompileSourceAndCreateRunner(session, source, digest, ref luaScriptHandle, out runner, out digestOnHeap); } /// /// Load a script previously stored in the global cache. /// - internal bool TryLoadCached(RespServerSession session, ScriptHashKey digest, LuaScriptHandle cachedScriptHandle, out LuaRunner runner) + internal bool TryGetOrCreateRunnerFromCachedScript(RespServerSession session, ScriptHashKey digest, LuaScriptHandle cachedScriptHandle, out LuaRunner runner) { if (TryGetFromDigest(digest, out runner, out _)) return true; return cachedScriptHandle.Chunk.Kind == LuaScriptChunkKind.GarnetGeneratedBinary - ? TryLoadCompiled(session, cachedScriptHandle.Chunk, digest, ref cachedScriptHandle, out runner, out _) - : TryCompileAndLoad(session, cachedScriptHandle.ScriptData.Span, digest, ref cachedScriptHandle, out runner, out _); + ? TryGetOrCreateRunnerFromGeneratedBytecode(session, cachedScriptHandle.Chunk, digest, ref cachedScriptHandle, out runner, out _) + : TryCompileSourceAndCreateRunner(session, cachedScriptHandle.ScriptData.Span, digest, ref cachedScriptHandle, out runner, out _); } - private bool TryCompileAndLoad(RespServerSession session, ReadOnlySpan source, ScriptHashKey digest, ref LuaScriptHandle luaScriptHandle, out LuaRunner runner, out ScriptHashKey? digestOnHeap) + private bool TryCompileSourceAndCreateRunner(RespServerSession session, ReadOnlySpan source, ScriptHashKey digest, ref LuaScriptHandle luaScriptHandle, out LuaRunner runner, out ScriptHashKey? digestOnHeap) { - LuaScriptChunk compiledSource; + LuaScriptChunk generatedBytecode; string error; try { - if (LuaRunner.TryCompileSource(source, out compiledSource, out error)) - return TryLoadCompiled(session, compiledSource, digest, ref luaScriptHandle, out runner, out digestOnHeap); + if (LuaRunner.TryCompileSource(source, out generatedBytecode, out error)) + return TryGetOrCreateRunnerFromGeneratedBytecode(session, generatedBytecode, digest, ref luaScriptHandle, out runner, out digestOnHeap); } catch (Exception ex) { @@ -227,7 +227,7 @@ private bool TryCompileAndLoad(RespServerSession session, ReadOnlySpan sou /// /// Load internally compiled script bytecode into the cache. /// - private bool TryLoadCompiled(RespServerSession session, LuaScriptChunk compiledSource, ScriptHashKey digest, ref LuaScriptHandle luaScriptHandle, out LuaRunner runner, out ScriptHashKey? digestOnHeap) + private bool TryGetOrCreateRunnerFromGeneratedBytecode(RespServerSession session, LuaScriptChunk generatedBytecode, ScriptHashKey digest, ref LuaScriptHandle luaScriptHandle, out LuaRunner runner, out ScriptHashKey? digestOnHeap) { if (TryGetFromDigest(digest, out runner, out var existingLuaScriptHandle)) { @@ -238,7 +238,7 @@ private bool TryLoadCompiled(RespServerSession session, LuaScriptChunk compiledS try { - runner = new LuaRunner(memoryManagementMode, memoryLimitBytes, logMode, allowedFunctions, compiledSource, storeWrapper.serverOptions.LuaTransactionMode, processor, scratchBufferNetworkSender, storeWrapper.redisProtocolVersion, logger); + runner = new LuaRunner(memoryManagementMode, memoryLimitBytes, logMode, allowedFunctions, generatedBytecode, storeWrapper.serverOptions.LuaTransactionMode, processor, scratchBufferNetworkSender, storeWrapper.redisProtocolVersion, logger); // If compilation fails, an error is written out if (runner.CompileForSession(session)) @@ -254,7 +254,7 @@ private bool TryLoadCompiled(RespServerSession session, LuaScriptChunk compiledS ScriptHashKey storeKeyDigest = new(into); digestOnHeap = storeKeyDigest; - luaScriptHandle ??= new(compiledSource); + luaScriptHandle ??= new(generatedBytecode); scriptCache.Add(storeKeyDigest, (runner, luaScriptHandle)); // On first script load, register for timeout notifications From e56277fb1e1013e264433265c99d5d7e9dc53988 Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 12:14:18 -0700 Subject: [PATCH 23/27] Deduplicate Lua out-of-memory responses Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- libs/server/Lua/LuaRunner.Functions.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/libs/server/Lua/LuaRunner.Functions.cs b/libs/server/Lua/LuaRunner.Functions.cs index dc02f30e06f..a82cd8c96ad 100644 --- a/libs/server/Lua/LuaRunner.Functions.cs +++ b/libs/server/Lua/LuaRunner.Functions.cs @@ -3083,17 +3083,13 @@ private unsafe int CompileCommon(nint luaState, ref TResponse resp) _ = state.RawGetInteger(LuaType.Function, (int)LuaRegistry.Index, loadSandboxedRegistryIndex); if (!state.TryPushBuffer(source.Data.Span)) { - while (!RespWriteUtils.TryWriteError(CmdStrings.LUA_out_of_memory, ref resp.BufferCur, resp.BufferEnd)) - resp.SendAndReset(); - + WriteOutOfMemoryError(ref resp); return 0; } if (!state.TryPushBuffer(source.Kind == LuaScriptChunkKind.GarnetGeneratedBinary ? "b"u8 : "t"u8)) { - while (!RespWriteUtils.TryWriteError(CmdStrings.LUA_out_of_memory, ref resp.BufferCur, resp.BufferEnd)) - resp.SendAndReset(); - + WriteOutOfMemoryError(ref resp); return 0; } @@ -3112,9 +3108,7 @@ private unsafe int CompileCommon(nint luaState, ref TResponse resp) if (!state.TryRef(out functionRegistryIndex)) { // Uh-oh, couldn't save the function under the registry - while (!RespWriteUtils.TryWriteError(CmdStrings.LUA_out_of_memory, ref resp.BufferCur, resp.BufferEnd)) - resp.SendAndReset(); - + WriteOutOfMemoryError(ref resp); return 0; } } @@ -3139,6 +3133,12 @@ private unsafe int CompileCommon(nint luaState, ref TResponse resp) return 0; } + private static unsafe void WriteOutOfMemoryError(ref TResponse resp) where TResponse : struct, IResponseAdapter + { + while (!RespWriteUtils.TryWriteError(CmdStrings.LUA_out_of_memory, ref resp.BufferCur, resp.BufferEnd)) + resp.SendAndReset(); + } + /// /// Entry point method for executing commands from a Lua Script /// From 70650ee49227ea1f9f45d2f41b75b2b3fa675c9b Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 12:32:49 -0700 Subject: [PATCH 24/27] Verify Lua parser error extraction Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs b/test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs index c43eae89f48..d12f4ebab6a 100644 --- a/test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs +++ b/test/standalone/Garnet.test.scripting/LuaScriptRunnerTests.cs @@ -172,7 +172,7 @@ public void TryCompileSourceRejectsBinaryAndInvalidInput() var invalidSource = "return )"u8; ClassicAssert.IsFalse(LuaRunner.TryCompileSource(invalidSource, out var rejectedSource, out var sourceError)); ClassicAssert.AreEqual(default(LuaScriptChunk), rejectedSource); - ClassicAssert.IsNotEmpty(sourceError); + StringAssert.Contains("unexpected symbol", sourceError); } [Test] From 37695466c57098e20d977b3e76df14533d811d36 Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 12:35:11 -0700 Subject: [PATCH 25/27] Clarify Lua stack error helper Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- libs/server/Lua/LuaRunner.Loader.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libs/server/Lua/LuaRunner.Loader.cs b/libs/server/Lua/LuaRunner.Loader.cs index 59751f19cdc..c7e23f061d5 100644 --- a/libs/server/Lua/LuaRunner.Loader.cs +++ b/libs/server/Lua/LuaRunner.Loader.cs @@ -499,7 +499,7 @@ internal static bool TryCompileSource(ReadOnlySpan source, out LuaScriptCh if (state.LoadTextBuffer(source) != LuaStatus.OK) { compiledSource = default; - error = GetError(state); + error = GetErrorFromStackTop(state); return false; } @@ -508,7 +508,7 @@ internal static bool TryCompileSource(ReadOnlySpan source, out LuaScriptCh if (state.PCall(2, 1) != LuaStatus.OK) { compiledSource = default; - error = GetError(state); + error = GetErrorFromStackTop(state); return false; } @@ -518,7 +518,7 @@ internal static bool TryCompileSource(ReadOnlySpan source, out LuaScriptCh error = null; return true; - static string GetError(LuaStateWrapper state) + static string GetErrorFromStackTop(LuaStateWrapper state) { var errorIndex = state.StackTop; if (errorIndex >= 1 && state.Type(errorIndex) == LuaType.String) From 514e05a24a307b50c8b4cdd3a2a43d0395cbcbf9 Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 12:45:16 -0700 Subject: [PATCH 26/27] Require generated bytecode in script cache Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- .../Lua/LuaScriptCacheOperations.cs | 12 ++++- libs/server/Lua/LuaCommands.cs | 51 +++++++++++++++---- libs/server/Lua/LuaScriptHandle.cs | 11 +--- libs/server/Lua/SessionScriptCache.cs | 30 ++++------- 4 files changed, 62 insertions(+), 42 deletions(-) diff --git a/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs b/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs index be4ec23aea9..df9dbf50741 100644 --- a/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs +++ b/benchmark/BDN.benchmark/Lua/LuaScriptCacheOperations.cs @@ -51,20 +51,28 @@ public void GlobalSetup() outerHitDigest = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); sessionScriptCache.GetScriptDigest("return 1"u8, outerHitDigest); - if (!storeWrapper.storeScriptCache.TryAdd(new(outerHitDigest), LuaScriptHandle.FromTextSource("return 1"u8.ToArray()))) + if (!storeWrapper.storeScriptCache.TryAdd(new(outerHitDigest), CompileScript("return 1"u8))) { throw new InvalidOperationException("Should have been able to load into global cache"); } innerHitDigest = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); sessionScriptCache.GetScriptDigest("return 1 + 1"u8, innerHitDigest); - if (!storeWrapper.storeScriptCache.TryAdd(new(innerHitDigest), LuaScriptHandle.FromTextSource("return 1 + 1"u8.ToArray()))) + if (!storeWrapper.storeScriptCache.TryAdd(new(innerHitDigest), CompileScript("return 1 + 1"u8))) { throw new InvalidOperationException("Should have been able to load into global cache"); } missDigest = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); sessionScriptCache.GetScriptDigest("foobar"u8, missDigest); + + static LuaScriptHandle CompileScript(ReadOnlySpan source) + { + if (!LuaRunner.TryCompileSource(source, out var generatedBytecode, out var error)) + throw new InvalidOperationException(error); + + return new(generatedBytecode.Data); + } } [GlobalCleanup] diff --git a/libs/server/Lua/LuaCommands.cs b/libs/server/Lua/LuaCommands.cs index b60648b3367..c9368c49a4a 100644 --- a/libs/server/Lua/LuaCommands.cs +++ b/libs/server/Lua/LuaCommands.cs @@ -48,12 +48,20 @@ private unsafe bool TryEVALSHA() { if (storeWrapper.storeScriptCache.TryGetValue(scriptKey, out var globalScriptHandle)) { - if (!sessionScriptCache.TryGetOrCreateRunnerFromCachedScript(this, scriptKey, globalScriptHandle, out runner)) + try { - // The loading error was already written, if any - // - // Note we DON'T dispose the script handle because this is just the session cache - _ = storeWrapper.storeScriptCache.TryRemove(scriptKey, out _); + if (!sessionScriptCache.TryGetOrCreateRunnerFromCachedScript(this, scriptKey, globalScriptHandle, out runner)) + { + // The loading error was already written, if any + // + // Note we DON'T dispose the script handle because this is just the session cache + _ = storeWrapper.storeScriptCache.TryRemove(scriptKey, out _); + return true; + } + } + catch (Exception ex) + { + WriteLuaInternalError(ex); return true; } } @@ -114,8 +122,17 @@ private unsafe bool TryEVAL() sessionScriptCache.GetScriptDigest(script.ReadOnlySpan, digest); var onStackScriptKey = new ScriptHashKey(digest); - if (!TryGetOrCreateScriptRunner(script.ReadOnlySpan, digest, onStackScriptKey, out var runner)) - return true; // The loading error was written to the response. + LuaRunner runner; + try + { + if (!TryGetOrCreateScriptRunner(script.ReadOnlySpan, digest, onStackScriptKey, out runner)) + return true; // The loading error was written to the response. + } + catch (Exception ex) + { + WriteLuaInternalError(ex); + return true; + } if (runner == null) { @@ -240,10 +257,17 @@ private bool NetworkScriptLoad() sessionScriptCache.GetScriptDigest(source.Span, digest); var onStackScriptHashKey = new ScriptHashKey(digest); - if (TryGetOrCreateScriptRunner(source.ReadOnlySpan, digest, onStackScriptHashKey, out _)) + try { - while (!RespWriteUtils.TryWriteBulkString(digest, ref dcurr, dend)) - SendAndReset(); + if (TryGetOrCreateScriptRunner(source.ReadOnlySpan, digest, onStackScriptHashKey, out _)) + { + while (!RespWriteUtils.TryWriteBulkString(digest, ref dcurr, dend)) + SendAndReset(); + } + } + catch (Exception ex) + { + WriteLuaInternalError(ex); } return true; @@ -299,6 +323,13 @@ internal void WriteLuaCompilationError(string error) SendAndReset(); } + private void WriteLuaInternalError(Exception ex) + { + logger?.LogError(ex, "Unexpected error while preparing Lua script"); + while (!RespWriteUtils.TryWriteError("ERR Internal Lua error"u8, ref dcurr, dend)) + SendAndReset(); + } + /// /// Run a resolved script for the current session. /// diff --git a/libs/server/Lua/LuaScriptHandle.cs b/libs/server/Lua/LuaScriptHandle.cs index adcc165a587..d93a688467c 100644 --- a/libs/server/Lua/LuaScriptHandle.cs +++ b/libs/server/Lua/LuaScriptHandle.cs @@ -27,20 +27,13 @@ public sealed class LuaScriptHandle : IDisposable public bool IsDisposed { get; private set; } /// - /// Source or internally compiled data for the associated Lua script. + /// Internally generated bytecode for the associated Lua script. /// public ReadOnlyMemory ScriptData => Chunk.Data; internal LuaScriptChunk Chunk { get; } - /// - /// Creates a handle for Lua source text. - /// - /// Lua source text. - /// A handle containing Lua source text. - public static LuaScriptHandle FromTextSource(ReadOnlyMemory scriptData) => new(new LuaScriptChunk(scriptData, LuaScriptChunkKind.Text)); - - internal LuaScriptHandle(LuaScriptChunk chunk) => Chunk = chunk; + internal LuaScriptHandle(ReadOnlyMemory generatedBytecode) => Chunk = new(generatedBytecode, LuaScriptChunkKind.GarnetGeneratedBinary); /// public void Dispose() diff --git a/libs/server/Lua/SessionScriptCache.cs b/libs/server/Lua/SessionScriptCache.cs index 0d9a12cebc6..336b2d95b8f 100644 --- a/libs/server/Lua/SessionScriptCache.cs +++ b/libs/server/Lua/SessionScriptCache.cs @@ -195,28 +195,15 @@ internal bool TryGetOrCreateRunnerFromCachedScript(RespServerSession session, Sc if (TryGetFromDigest(digest, out runner, out _)) return true; - return cachedScriptHandle.Chunk.Kind == LuaScriptChunkKind.GarnetGeneratedBinary - ? TryGetOrCreateRunnerFromGeneratedBytecode(session, cachedScriptHandle.Chunk, digest, ref cachedScriptHandle, out runner, out _) - : TryCompileSourceAndCreateRunner(session, cachedScriptHandle.ScriptData.Span, digest, ref cachedScriptHandle, out runner, out _); + return TryGetOrCreateRunnerFromGeneratedBytecode(session, cachedScriptHandle.Chunk, digest, ref cachedScriptHandle, out runner, out _); } private bool TryCompileSourceAndCreateRunner(RespServerSession session, ReadOnlySpan source, ScriptHashKey digest, ref LuaScriptHandle luaScriptHandle, out LuaRunner runner, out ScriptHashKey? digestOnHeap) { LuaScriptChunk generatedBytecode; string error; - try - { - if (LuaRunner.TryCompileSource(source, out generatedBytecode, out error)) - return TryGetOrCreateRunnerFromGeneratedBytecode(session, generatedBytecode, digest, ref luaScriptHandle, out runner, out digestOnHeap); - } - catch (Exception ex) - { - logger?.LogError(ex, "During Lua script compilation, an unexpected exception"); - runner = null; - digestOnHeap = null; - luaScriptHandle = null; - return false; - } + if (LuaRunner.TryCompileSource(source, out generatedBytecode, out error)) + return TryGetOrCreateRunnerFromGeneratedBytecode(session, generatedBytecode, digest, ref luaScriptHandle, out runner, out digestOnHeap); session.WriteLuaCompilationError(error); runner = null; @@ -236,6 +223,7 @@ private bool TryGetOrCreateRunnerFromGeneratedBytecode(RespServerSession session return true; } + runner = null; try { runner = new LuaRunner(memoryManagementMode, memoryLimitBytes, logMode, allowedFunctions, generatedBytecode, storeWrapper.serverOptions.LuaTransactionMode, processor, scratchBufferNetworkSender, storeWrapper.redisProtocolVersion, logger); @@ -254,7 +242,7 @@ private bool TryGetOrCreateRunnerFromGeneratedBytecode(RespServerSession session ScriptHashKey storeKeyDigest = new(into); digestOnHeap = storeKeyDigest; - luaScriptHandle ??= new(generatedBytecode); + luaScriptHandle ??= new(generatedBytecode.Data); scriptCache.Add(storeKeyDigest, (runner, luaScriptHandle)); // On first script load, register for timeout notifications @@ -273,13 +261,13 @@ private bool TryGetOrCreateRunnerFromGeneratedBytecode(RespServerSession session return false; } } - catch (Exception ex) + catch { - logger?.LogError(ex, "During Lua script loading, an unexpected exception"); - + runner?.Dispose(); + runner = null; digestOnHeap = null; luaScriptHandle = null; - return false; + throw; } return true; From 4f8f299e9e703d4431b4209699d3477b856e683c Mon Sep 17 00:00:00 2001 From: Tiago Napoli Date: Wed, 16 Sep 2026 13:40:41 -0700 Subject: [PATCH 27/27] Restore Lua cache exception handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec2a6abf-4bb4-48e1-a752-32672cb07303 --- libs/server/Lua/LuaCommands.cs | 51 ++++++--------------------- libs/server/Lua/SessionScriptCache.cs | 24 +++++++++---- 2 files changed, 27 insertions(+), 48 deletions(-) diff --git a/libs/server/Lua/LuaCommands.cs b/libs/server/Lua/LuaCommands.cs index c9368c49a4a..b60648b3367 100644 --- a/libs/server/Lua/LuaCommands.cs +++ b/libs/server/Lua/LuaCommands.cs @@ -48,20 +48,12 @@ private unsafe bool TryEVALSHA() { if (storeWrapper.storeScriptCache.TryGetValue(scriptKey, out var globalScriptHandle)) { - try + if (!sessionScriptCache.TryGetOrCreateRunnerFromCachedScript(this, scriptKey, globalScriptHandle, out runner)) { - if (!sessionScriptCache.TryGetOrCreateRunnerFromCachedScript(this, scriptKey, globalScriptHandle, out runner)) - { - // The loading error was already written, if any - // - // Note we DON'T dispose the script handle because this is just the session cache - _ = storeWrapper.storeScriptCache.TryRemove(scriptKey, out _); - return true; - } - } - catch (Exception ex) - { - WriteLuaInternalError(ex); + // The loading error was already written, if any + // + // Note we DON'T dispose the script handle because this is just the session cache + _ = storeWrapper.storeScriptCache.TryRemove(scriptKey, out _); return true; } } @@ -122,17 +114,8 @@ private unsafe bool TryEVAL() sessionScriptCache.GetScriptDigest(script.ReadOnlySpan, digest); var onStackScriptKey = new ScriptHashKey(digest); - LuaRunner runner; - try - { - if (!TryGetOrCreateScriptRunner(script.ReadOnlySpan, digest, onStackScriptKey, out runner)) - return true; // The loading error was written to the response. - } - catch (Exception ex) - { - WriteLuaInternalError(ex); - return true; - } + if (!TryGetOrCreateScriptRunner(script.ReadOnlySpan, digest, onStackScriptKey, out var runner)) + return true; // The loading error was written to the response. if (runner == null) { @@ -257,17 +240,10 @@ private bool NetworkScriptLoad() sessionScriptCache.GetScriptDigest(source.Span, digest); var onStackScriptHashKey = new ScriptHashKey(digest); - try + if (TryGetOrCreateScriptRunner(source.ReadOnlySpan, digest, onStackScriptHashKey, out _)) { - if (TryGetOrCreateScriptRunner(source.ReadOnlySpan, digest, onStackScriptHashKey, out _)) - { - while (!RespWriteUtils.TryWriteBulkString(digest, ref dcurr, dend)) - SendAndReset(); - } - } - catch (Exception ex) - { - WriteLuaInternalError(ex); + while (!RespWriteUtils.TryWriteBulkString(digest, ref dcurr, dend)) + SendAndReset(); } return true; @@ -323,13 +299,6 @@ internal void WriteLuaCompilationError(string error) SendAndReset(); } - private void WriteLuaInternalError(Exception ex) - { - logger?.LogError(ex, "Unexpected error while preparing Lua script"); - while (!RespWriteUtils.TryWriteError("ERR Internal Lua error"u8, ref dcurr, dend)) - SendAndReset(); - } - /// /// Run a resolved script for the current session. /// diff --git a/libs/server/Lua/SessionScriptCache.cs b/libs/server/Lua/SessionScriptCache.cs index 336b2d95b8f..cfe38ab2bf2 100644 --- a/libs/server/Lua/SessionScriptCache.cs +++ b/libs/server/Lua/SessionScriptCache.cs @@ -202,8 +202,19 @@ private bool TryCompileSourceAndCreateRunner(RespServerSession session, ReadOnly { LuaScriptChunk generatedBytecode; string error; - if (LuaRunner.TryCompileSource(source, out generatedBytecode, out error)) - return TryGetOrCreateRunnerFromGeneratedBytecode(session, generatedBytecode, digest, ref luaScriptHandle, out runner, out digestOnHeap); + try + { + if (LuaRunner.TryCompileSource(source, out generatedBytecode, out error)) + return TryGetOrCreateRunnerFromGeneratedBytecode(session, generatedBytecode, digest, ref luaScriptHandle, out runner, out digestOnHeap); + } + catch (Exception ex) + { + logger?.LogError(ex, "During Lua script compilation, an unexpected exception"); + runner = null; + digestOnHeap = null; + luaScriptHandle = null; + return false; + } session.WriteLuaCompilationError(error); runner = null; @@ -223,7 +234,6 @@ private bool TryGetOrCreateRunnerFromGeneratedBytecode(RespServerSession session return true; } - runner = null; try { runner = new LuaRunner(memoryManagementMode, memoryLimitBytes, logMode, allowedFunctions, generatedBytecode, storeWrapper.serverOptions.LuaTransactionMode, processor, scratchBufferNetworkSender, storeWrapper.redisProtocolVersion, logger); @@ -261,13 +271,13 @@ private bool TryGetOrCreateRunnerFromGeneratedBytecode(RespServerSession session return false; } } - catch + catch (Exception ex) { - runner?.Dispose(); - runner = null; + logger?.LogError(ex, "During Lua script loading, an unexpected exception"); + digestOnHeap = null; luaScriptHandle = null; - throw; + return false; } return true;