From 00231c857b61dee1429e7d67d52221bcd9e8931b Mon Sep 17 00:00:00 2001
From: alpaca-tc <alpaca-tc@alpaca.tc>
Date: Thu, 26 Sep 2024 17:55:17 +0900
Subject: [PATCH] Fixed a bug that ConnectionPool holds the same Redis client.

When `pool_size` or `pool_timeout` is passed as an option, `Redis::Rack::Connection` builds a `ConnectionPool`.
`ConnectionPool` will execute a given block when there are not enough connections. However, since the calling store is cached in an instance variable, all connections held by the `ConnectionPool` are actually same objects.
---
 lib/redis/rack/connection.rb       | 10 ++++++++--
 test/redis/rack/connection_test.rb | 26 ++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/lib/redis/rack/connection.rb b/lib/redis/rack/connection.rb
index 569d5ff..fd77048 100644
--- a/lib/redis/rack/connection.rb
+++ b/lib/redis/rack/connection.rb
@@ -32,11 +32,11 @@ def pooled?
       end
 
       def pool
-        @pool ||= ConnectionPool.new(pool_options) { store } if pooled?
+        @pool ||= ConnectionPool.new(pool_options) { build_store } if pooled?
       end
 
       def store
-        @store ||= Redis::Store::Factory.create(@options[:redis_server])
+        @store ||= build_store
       end
 
       def pool_options
@@ -45,6 +45,12 @@ def pool_options
           timeout: @options[:pool_timeout]
         }.reject { |key, value| value.nil? }.to_h
       end
+
+      private
+
+      def build_store
+        Redis::Store::Factory.create(@options[:redis_server])
+      end
     end
   end
 end
diff --git a/test/redis/rack/connection_test.rb b/test/redis/rack/connection_test.rb
index ab66f6c..08a7faf 100644
--- a/test/redis/rack/connection_test.rb
+++ b/test/redis/rack/connection_test.rb
@@ -37,6 +37,32 @@ def setup
         end
       end
 
+      it "can establish a new connection pool with the provided Redis server" do
+        pool_size = 5
+        conn = Connection.new(redis_server: 'redis://127.0.0.1:6380/1', pool_size: pool_size)
+
+        clients = []
+        mutex = Mutex.new
+        threads = Array.new(pool_size) do |i|
+          Thread.new do
+            conn.with do |connection|
+              mutex.synchronize do
+                clients[i] = connection
+              end
+
+              # Wait for all clients to be established
+              loop do
+                break if clients.size == pool_size && clients.all?
+                sleep(0.01)
+              end
+            end
+          end
+        end.each(&:join)
+
+        conn.pooled?.must_equal true
+        clients.map(&:object_id).uniq.size.must_equal pool_size
+      end
+
       it "can use a supplied pool" do
         pool = ConnectionPool.new size: 1, timeout: 1 do
           ::Redis::Store::Factory.create('redis://127.0.0.1:6380/1')