From dca5927ac868ad697f646bcf9efc6d9b79afb03f Mon Sep 17 00:00:00 2001
From: Madelyn Olson <34459052+madolson@users.noreply.github.com>
Date: Tue, 21 Feb 2023 08:14:41 -0800
Subject: [PATCH] Prevent Redis from crashing from key tracking invalidations
 (#11814)

There is a built in limit to client side tracking keys, which when exceeded will invalidate keys. This occurs in two places, one in the server cron and other before executing a command. If it happens in the second scenario, the invalidations will be queued for later since current client is set. This queue is never drained if a command is not executed (through call) such as a multi-exec command getting queued. This results in a later server assert crashing.
---
 src/tracking.c          |  6 +++---
 tests/unit/tracking.tcl | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/src/tracking.c b/src/tracking.c
index 3d207563f..775eea684 100644
--- a/src/tracking.c
+++ b/src/tracking.c
@@ -393,10 +393,10 @@ void trackingInvalidateKey(client *c, robj *keyobj, int bcast) {
             continue;
         }
 
-        /* If target is current client, we need schedule key invalidation.
+        /* If target is current client and it's executing a command, we need schedule key invalidation.
          * As the invalidation messages may be interleaved with command
-         * response and should after command response */
-        if (target == server.current_client){
+         * response and should after command response. */
+        if (target == server.current_client && (server.current_client->flags & CLIENT_EXECUTING_COMMAND)) {
             incrRefCount(keyobj);
             listAddNodeTail(server.tracking_pending_keys, keyobj);
         } else {
diff --git a/tests/unit/tracking.tcl b/tests/unit/tracking.tcl
index f4e6e27ab..13ca8f278 100644
--- a/tests/unit/tracking.tcl
+++ b/tests/unit/tracking.tcl
@@ -742,6 +742,44 @@ start_server {tags {"tracking network"}} {
         assert_equal {} $prefixes
     }
 
+    test {Regression test for #11715} {
+        # This issue manifests when a client invalidates keys through the max key
+        # limit, which invalidates keys to get Redis below the limit, but no command is
+        # then executed. This can occur in several ways but the simplest is through 
+        # multi-exec which queues commands.
+        clean_all
+        r config set tracking-table-max-keys 2
+
+        # The cron will invalidate keys if we're above the limit, so disable it.
+        r debug pause-cron 1
+
+        # Set up a client that has listened to 2 keys and start a multi, this
+        # sets up the crash for later.
+        $rd HELLO 3
+        $rd read
+        $rd CLIENT TRACKING on
+        assert_match "OK" [$rd read]
+        $rd mget "1{tag}" "2{tag}"
+        assert_match "{} {}" [$rd read]
+        $rd multi
+        assert_match "OK" [$rd read]
+
+        # Reduce the tracking table keys to 1, this doesn't immediately take affect, but
+        # instead will apply on the next command.
+        r config set tracking-table-max-keys 1
+
+        # This command will get queued, so make sure this command doesn't crash.
+        $rd ping
+        $rd exec
+
+        # Validate we got some invalidation message and then the command was queued.
+        assert_match "invalidate *{tag}" [$rd read]
+        assert_match "QUEUED" [$rd read]
+        assert_match "PONG" [$rd read]
+
+        r debug pause-cron 0
+    } {OK} {needs:debug}
+
     $rd_redirection close
     $rd close
 }