From a33197858320fd980faeab2ae721144d9b5afa4b Mon Sep 17 00:00:00 2001
From: Oran Agra <oran@redislabs.com>
Date: Wed, 17 Jul 2024 15:42:28 +0300
Subject: [PATCH] Fix external test hang in redis-cli test when run in a
 certain order (#13423)

When the tests are run against an external server in this order:
`--single unit/introspection --single unit/moduleapi/blockonbackground
--single integration/redis-cli`
the test would hang when the "ASK redirect test" test attempts to create
a listening socket (it fails, and then redis-cli itself hangs waiting
for a non-responsive socket created by the introspection test).

the reasons are:
1. the blockedbackground test includes util.tcl and resets the
`::last_port_attempted` variable
2. the test in introspection didn't close the listening server, so it's
still alive.
3. find_available_port doesn't properly detect the busy port, and it
thinks that the port is free even though it's busy.

fixing all 3 of these problems, even though fixing just one would be
enough to let the test pass.
---
 tests/helpers/fake_redis_node.tcl          | 4 +++-
 tests/support/util.tcl                     | 5 +++--
 tests/unit/introspection.tcl               | 3 ++-
 tests/unit/moduleapi/blockonbackground.tcl | 2 --
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/tests/helpers/fake_redis_node.tcl b/tests/helpers/fake_redis_node.tcl
index a12d87fedf..0f69d0a43d 100644
--- a/tests/helpers/fake_redis_node.tcl
+++ b/tests/helpers/fake_redis_node.tcl
@@ -53,6 +53,8 @@ proc accept {sock host port} {
     close $sock
 }
 
-socket -server accept $port
+set sockfd [socket -server accept -myaddr 127.0.0.1 $port]
 after 5000 set done timeout
 vwait done
+close $sockfd
+
diff --git a/tests/support/util.tcl b/tests/support/util.tcl
index bb07266a4e..858fc5a552 100644
--- a/tests/support/util.tcl
+++ b/tests/support/util.tcl
@@ -485,8 +485,9 @@ proc find_available_port {start count} {
             set port $start
         }
         set fd1 -1
-        if {[catch {set fd1 [socket -server 127.0.0.1 $port]}] ||
-            [catch {set fd2 [socket -server 127.0.0.1 [expr $port+10000]]}]} {
+        proc dummy_accept {chan addr port} {}
+        if {[catch {set fd1 [socket -server dummy_accept -myaddr 127.0.0.1 $port]}] ||
+            [catch {set fd2 [socket -server dummy_accept -myaddr 127.0.0.1 [expr $port+10000]]}]} {
             if {$fd1 != -1} {
                 close $fd1
             }
diff --git a/tests/unit/introspection.tcl b/tests/unit/introspection.tcl
index 194ff09e00..fbd1d14fef 100644
--- a/tests/unit/introspection.tcl
+++ b/tests/unit/introspection.tcl
@@ -648,7 +648,7 @@ start_server {tags {"introspection"}} {
         # Run a dummy server on used_port so we know we can't configure redis to 
         # use it. It's ok for this to fail because that means used_port is invalid 
         # anyway
-        catch {socket -server dummy_accept -myaddr 127.0.0.1 $used_port} e
+        catch {set sockfd [socket -server dummy_accept -myaddr 127.0.0.1 $used_port]} e
         if {$::verbose} { puts "dummy_accept: $e" }
 
         # Try to listen on the used port, pass some more configs to make sure the
@@ -670,6 +670,7 @@ start_server {tags {"introspection"}} {
         set r1 [redis_client]
         assert_equal [$r1 ping] "PONG"
         $r1 close
+        close $sockfd
     }
 
     test {CONFIG SET duplicate configs} {
diff --git a/tests/unit/moduleapi/blockonbackground.tcl b/tests/unit/moduleapi/blockonbackground.tcl
index fcd7f1dd44..7220ea57c9 100644
--- a/tests/unit/moduleapi/blockonbackground.tcl
+++ b/tests/unit/moduleapi/blockonbackground.tcl
@@ -1,7 +1,5 @@
 set testmodule [file normalize tests/modules/blockonbackground.so]
 
-source tests/support/util.tcl
-
 proc latency_percentiles_usec {cmd} {
     return [latencyrstat_percentiles $cmd r]
 }