Skip to content

Conversation

@oranagra
Copy link
Member

@oranagra oranagra commented Sep 3, 2024

When a client in no-touch mode issues a TOUCH command on a key, the key’s access time should be updated, but in scripts, and module's RM_Call, it isn’t updated.
Command proc should be matched to the executing client, not the current client.

Command proc should be matched to the executing client, not the current
client.

Co-authored-by: Udi Ron <udi@speedb.io>
@oranagra oranagra requested a review from sundb September 3, 2024 11:23
Copy link
Collaborator

@sundb sundb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@oranagra oranagra merged commit 610eb26 into redis:unstable Sep 12, 2024
@oranagra oranagra deleted the fix_touch_no_touch branch September 12, 2024 08:33
@YaacovHazan YaacovHazan added the release-notes indication that this issue needs to be mentioned in the release notes label Sep 26, 2024
@sundb sundb added this to Redis 8.0 Aug 15, 2025
@sundb sundb removed this from Redis 8.2 Aug 15, 2025
@sundb sundb moved this from Todo to Done in Redis 8.0 Aug 15, 2025
funny-dog pushed a commit to funny-dog/redis that referenced this pull request Sep 17, 2025
When a client in no-touch mode issues a TOUCH command on a key, the
key’s access time should be updated, but in scripts, and module's
RM_Call, it isn’t updated.
Command proc should be matched to the executing client, not the current
client.

Co-authored-by: Udi Ron <udi@speedb.io>
moticless added a commit that referenced this pull request Oct 13, 2025
This PR is based on:
valkey-io/valkey#2347

This was introduced in #13512

The server crashes with a null pointer dereference when lookupKey() is
called from handleClientsBlockedOnKey(). The crash occurs because
server.executing_client is NULL, but the code attempts to access
server.executing_client->cmd->proc without checking.

**Crash scenario:**
Client 1 enables CLIENT NO-TOUCH
Client 2 blocks on BRPOP mylist 0
Client 1 executes RPUSH mylist elem
When unblocking Client 2, lookupKey() dereferences NULL
server.executing_client → crash

**Solution**
Added proper null checks before dereferencing server.executing_client:
Check if LOOKUP_NOTOUCH flag is already set before attempting to modify
it
Verify both server.current_client and server.executing_client are not
NULL before accessing their members
Maintain the TOUCH command exception for scripts

**Testing**
Added regression test in tests/unit/type/list.tcl that reproduces and
verifies the fix for this crash scenario.

This fix is based on valkey-io/valkey#2347

Co-authored-by: Uri Yagelnik <uriy@amazon.com>
Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes indication that this issue needs to be mentioned in the release notes

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants