[Bugfix] SortedSet Race Condition - Mutating under Shared Lock

[hamdaankhalid]

TLDR:
SortedSetTimeToLive/ZTTL uses the Read Tsavorite method which uses Shared Lock, and then in SortedSetObjectImpl layer it was mutating shared data structure.

Reasoning:
As I understand, we handle thread safety for SortedSet data structures by depending on the state mutating methods like DeleteExpiredItem to be called for commands under the ExclusiveLock commands using Tsavorite Upsert or RMW methods.
If a SortedSet command is using the Read methods and then it mutates state, that breaks the invariant.

[Copilot]

Pull request overview

Fixes a SortedSet thread-safety issue where ZTTL/ZPTTL (read-path operations) mutated SortedSet state by calling DeleteExpiredItems() under Tsavorite’s shared/read lock, breaking the object-store locking invariant. Adds a new test intended to reproduce a related race involving shallow cloning during flush serialization.

Changes:

• Remove DeleteExpiredItems() from SortedSetTimeToLive to prevent mutations on the read path.
• Add SortedSetShallowCloneRaceTests to exercise a flush + CopyUpdater shallow-clone concurrency scenario.

Reviewed changes

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

File	Description
libs/server/Objects/SortedSet/SortedSetObjectImpl.cs	Stops ZTTL from mutating SortedSet internals during read operations by removing DeleteExpiredItems().
test/Garnet.test/SortedSetShallowCloneRaceTests.cs	Introduces a new race-repro test intended to demonstrate shallow-clone mutation racing with serialization during flush.

Comments suppressed due to low confidence (1)

libs/server/Objects/SortedSet/SortedSetObjectImpl.cs:868

• Removing DeleteExpiredItems() avoids mutating under a shared/read lock, but SortedSetTimeToLive now treats an expired member like a non-expiring member: GetExpiration returns the stored ticks, and ConvertUtils.*FromDiffUtcNowTicks returns -1 once the timestamp is in the past. That conflates “expired” with “no expiration”, and can change ZTTL/ZPTTL semantics. Consider returning -2 for expired members without mutating state (e.g., check IsExpired(member) or compare the returned expiration ticks to UtcNow and treat as not found).

        private void SortedSetTimeToLive(ref ObjectInput input, ref GarnetObjectStoreOutput output, byte respProtocolVersion)
        {
            var isMilliseconds = input.arg1 == 1;
            var isTimestamp = input.arg2 == 1;
            var numFields = input.parseState.Count;

            using var writer = new RespMemoryWriter(respProtocolVersion, ref output.SpanByteAndMemory);

            writer.WriteArrayLength(numFields);

            foreach (var item in input.parseState.Parameters)
            {
                var result = GetExpiration(item.ToArray());

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.