madelson / DistributedLock

A .NET library for distributed synchronization
MIT License
1.86k stars 192 forks source link

Redis: RedisScript (LuaScript) execution error - CultureInfo problem #162

Closed bsr-ky closed 6 months ago

bsr-ky commented 1 year ago

There is a CultureInfo problem when executing RedisScript (LuaScript). For CulturerInfo "tr-TR", a parameter named "@lockId" cannot be found in script text when disposing lock.

Reason: The cause of issue is culture dependent regex named as ParameterExtractor at StackExchange.Redis. The issue (https://github.com/StackExchange/StackExchange.Redis/issues/2350) is fixed at version 2.6.96 (https://stackexchange.github.io/StackExchange.Redis/ReleaseNotes).

Solution: Version of StackExchange.Redis should be upgraded from 2.2.4 to 2.6.96.

Error Stack Trace:

System.ArgumentException: ps (Parameter 'Expected [lock] to be a field or gettable property on [<>fAnonymousType02[[StackExchange.Redis.RedisKey, StackExchange.Redis, Version=2.0.0.0, Culture=neutral, PublicKeyToken=c219ff1ca8c2ce46],[StackExchange.Redis.RedisValue, StackExchange.Redis, Version=2.0.0.0, Culture=neutral, PublicKeyToken=c219ff1ca8c2ce46]]]') at StackExchange.Redis.LuaScript.ExtractParameters(Object ps, Nullable1 keyPrefix, RedisKey[]& keys, RedisValue[]& args) in //src/StackExchange.Redis/LuaScript.cs:line 117 at StackExchange.Redis.LuaScript.Evaluate(IDatabase db, Object ps, Nullable`1 withKeyPrefix, CommandFlags flags) in //src/StackExchange.Redis/LuaScript.cs:line 148 at StackExchange.Redis.RedisDatabase.ScriptEvaluate(LuaScript script, Object parameters, CommandFlags flags) in //src/StackExchange.Redis/RedisDatabase.cs:line 1223 at Medallion.Threading.Redis.Primitives.RedisScript`1.Execute(IDatabase database, TArgument argument, Boolean fireAndForget) in //DistributedLock.Redis/Primitives/RedisScript.cs:line 25 at Medallion.Threading.Redis.Primitives.RedisMutexPrimitive.Release(IDatabase database, Boolean fireAndForget) in //DistributedLock.Redis/Primitives/RedisMutexPrimitive.cs:line 35 at Medallion.Threading.Redis.RedLock.RedLockRelease.ReleaseAsync() in //DistributedLock.Redis/RedLock/RedLockRelease.cs:line 68 --- End of inner exception stack trace --- at Medallion.Threading.Redis.RedLock.RedLockRelease.ReleaseAsync() in //DistributedLock.Redis/RedLock/RedLockRelease.cs:line 78 at Medallion.Threading.Redis.RedLock.RedLockHandle.DisposeAsync() in //DistributedLock.Redis/RedLock/RedLockHandle.cs:line 52 at Medallion.Threading.Internal.SyncViaAsync.<>c3`1.<b__30>d.MoveNext() in //DistributedLock.Core/Internal/SyncViaAsync.cs:line 35

madelson commented 1 year ago

Thanks for reporting @bsr-ky ! I will include the upgraded version in the next release.

In the meantime, can you confirm that you can resolve this issue by adding a PackageReference to the higher version in your project?

bsr-ky commented 1 year ago

Yes, I can confirm. I wrote a simple unit test for validation but could not push to a remote branch in this repository because of permission error.

madelson commented 1 year ago

I wrote a simple unit test for validation but could not push to a remote branch in this repository because of permission error.

To make a PR, you should be able to fork this repo, push the change to a branch in your fork, and then make a PR back to my repo. I'd love to take a look at it.

madelson commented 1 year ago

I tried updating to both this version and the latest version (2.6.122); I'm seeing some test failures. I'll need to investigate further.

jrgcubano commented 1 year ago

Hello @madelson. Any news in this regard? Thanks in advance!

madelson commented 1 year ago

@jrgcubano is the workaround of just upgrading stackexchange.redis yourself not working for you?

I do intend to get to this, but haven’t had time recently. My impression is that no one is blocked and so this is more of an inconvenience.

jrgcubano commented 1 year ago

@madelson Sorry for the delay in answering.

I interpreted the issue incorrectly. Everything works perfectly and the upgrade too.

Thanks a lot for this library.