-
Notifications
You must be signed in to change notification settings - Fork 502
-
Notifications
You must be signed in to change notification settings - Fork 502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NullReferenceException in ExecuteScript Method When Executing Lua Scripts #635
Comments
I'm seeing the same error when trying to use Redisson maps. Dug down to the following line in garnet/libs/server/Lua/LuaCommands.cs Line 38 in f281bc3
which does not return a value. Looking at |
Digging into my case further, it is correct that there is no value returned. If I run with Redis I get a NOSCRIPT error back on the same execution. So I may be running into something different. |
Looks like this is at least part of the problem: garnet/libs/server/Lua/LuaCommands.cs Line 235 in f281bc3
ex.innerException is null.Changing that line to while (!RespWriteUtils.WriteError("ERR " + (ex.InnerException ?? ex).Message, ref dcurr, dend)) fixes the crash and forwards the error on to the client. |
The fix LGTM, if you can create a PR that would be great. |
@danielchristianschroeter - please check whether this change unblocks your example. |
Created #642 |
@mmclure-msft
|
I'm seeing similar issues. I think there may be subtle (or not so subtle) differences in how NLua parses the scripts vs. the library that's used by Redis. Setting
But I think it might be best to track this under a new issue once we have more details. |
Thank you! I already tried setting the logger level to Trace, but I still don’t get additional information in my case. All I receive is:
Without more details, it’s hard to debug. But of course, we can close this issue if the PR is merged. |
After adding detailed logging to the ExecuteScript function within libs/server/Lua/LuaCommands.cs, I identified that the Lua script is attempting to call a function named status_reply, which is not defined. Log Output:
Relevant Code: garnet/libs/server/Lua/LuaCommands.cs Line 192 in f281bc3
|
I think the main issue is that much of the API documented at https://redis.io/docs/latest/develop/interact/programmability/lua-api/ is not yet implemented. It will probably require community involvement to add the functionality to bring Garnet Lua scripting to parity with Redis. Definitely not a couple of small PRs 😄 |
|
@mmclure-msft Log Output: Even though the Lua script executes successfully, my code still fails with To fix this, in addition to your suggested changes, I modified the code in libs/server/Lua/LuaRunner.cs to ensure the Lua script returns a string, as required for compatibility with the original Redis code (referencing this line). Here’s the quick and dirty test approach I used:
Log Output After Changes: After applying these changes, the code now executes successfully. |
Interesting. Per https://redis.io/docs/latest/develop/interact/programmability/lua-api it's supposed to return a table - I'll need to experiment with real Redis and see what happens. |
I see your point, but for some reason the same code is working fine with Redis v7.4.0. |
With a version of your code I have this almost working the same as Redis for both
in Redis vs.
in Garnet (ignore the timings - that's just because I had breakpoints enabled). The difference is |
Added PR #645 to handle this correctly. |
Thank you! This looks good, and the blocking issue appears to be resolved. However, I've encountered a new undesired behavior: I'm getting a "lock not held" error during the Release, although this doesn't cause any blocking behavior. I previously also set "LuaTransactionMode" to "false." Here’s an example code:
For reference, here’s the related function in |
I'm pretty sure the issue is with this line in |
Actually, garnet/libs/server/Lua/LuaRunner.cs Line 68 in b92c3dc
Will dig a little deeper. |
For obtain.lua, I am hitting an index out of bounds issue with the This seems to be an open issue in NLua, as per this: NLua/NLua#472 |
This unit test works: https://github.com/microsoft/garnet/blob/badrishc/lua-more-fns/test/Garnet.test/LuaScriptTests.cs#L352 But if I replace "a" with "KEYS" which is populated with 1 based offsets, the ipairs fails. This is what obtain.lua does:
|
I worked around the |
It seems a more robust solution would be to make KEYS and ARGV LuaTable types rather than C# string arrays. That way, all Lua functions on them would be mostly guaranteed to work. |
The linked PR #672 should support most existing Lua scripts - let me know if you see any issues. |
Thanks! |
Good news - this is now fixed in the latest commit to my linked PR. The example from https://github.com/bsm/redislock?tab=readme-ov-file#examples now works unmodified. If your code still does not work, please provide a stand-alone example similar to the redislock example I linked, so that I can run it directly using |
@badrishc, A huge thank you to both you and @mmclure-msft for your tremendous effort and time invested in resolving these issues. It’s wonderful to see the progress that's been made! Let's close this issue if the PR is merged. |
@badrishc It seems, this part is not working as expected:
Response with Garnet:
Response with Redis (Docker image redis:latest):
Update: Slightly modified main.go with additional logging:
With Garnet:
With Redis:
|
Describe the bug
When attempting to execute Lua scripts using GarnetServer, a NullReferenceException is thrown in the ExecuteScript method. This issue occurs when using the EVAL or EVALSHA commands.
A NullReferenceException is thrown, indicating that an object reference is not set to an instance of an object.
Error Message:
The LuaRunner object might not be properly initialized or retrieved, leading to a null reference when attempting to execute the script.
This report is related to the Lua implementation in #550
Steps to reproduce the bug
Go Code Using RedisLock from https://github.com/bsm/redislock/blob/main/redislock.go:
Related Lua scripts:
https://github.com/bsm/redislock/blob/main/obtain.lua
https://github.com/bsm/redislock/blob/main/refresh.lua
Expected behavior
The Lua script should execute without throwing a NullReferenceException.
Screenshots
No response
Release version
v1.0.19
IDE
No response
OS version
No response
Additional context
Docker Compose service configuration:
docker-compose.yml
garnet.conf
The text was updated successfully, but these errors were encountered: