From 0e2477b602b3aa5b66c849d19737a8b66c73f633 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue, 29 Nov 2011 15:04:41 -0800
Subject: [PATCH] Automatic closure of connections in threads is deprecated. 
 For example the following code is deprecated:

Thread.new { Post.find(1) }.join

It should be changed to close the database connection at the end of
the thread:

Thread.new {
  Post.find(1)
  Post.connection.close
}.join

Only people who spawn threads in their application code need to worry
about this change.
---
 activerecord/CHANGELOG.md                     | 16 ++++++++++
 .../abstract/connection_pool.rb               |  8 ++++-
 .../test/cases/connection_pool_test.rb        | 31 +++----------------
 activerecord/test/cases/transactions_test.rb  |  3 ++
 4 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 62dc19a988..1a95b8e95e 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,5 +1,21 @@
 ## Rails 3.2.0 (unreleased) ##
 
+*   Automatic closure of connections in threads is deprecated.  For example
+    the following code is deprecated:
+
+    Thread.new { Post.find(1) }.join
+
+    It should be changed to close the database connection at the end of
+    the thread:
+
+    Thread.new {
+      Post.find(1)
+      Post.connection.close
+    }.join
+    
+    Only people who spawn threads in their application code need to worry
+    about this change.
+
 *   Deprecated:
 
       * `set_table_name`
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 656073b47a..d127b3ebd8 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -190,7 +190,13 @@ def clear_stale_cached_connections!
           t.alive?
         }.map { |thread| thread.object_id }
         keys.each do |key|
-          checkin @reserved_connections[key]
+          conn = @reserved_connections[key]
+          ActiveSupport::Deprecation.warn(<<-eowarn) if conn.in_use?
+Database connections will not be closed automatically, please close your
+database connection at the end of the thread by calling `close` on your
+connection.  For example: ActiveRecord::Base.connection.close
+          eowarn
+          checkin conn
           @reserved_connections.delete(key)
         end
       end
diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb
index 1550fa5530..d170a13b23 100644
--- a/activerecord/test/cases/connection_pool_test.rb
+++ b/activerecord/test/cases/connection_pool_test.rb
@@ -26,30 +26,6 @@ def test_active_connection?
         assert !@pool.active_connection?
       end
 
-      def test_clear_stale_cached_connections!
-        pool = ConnectionPool.new ActiveRecord::Base.connection_pool.spec
-
-        threads = [
-          Thread.new { pool.connection },
-          Thread.new { pool.connection }]
-
-        threads.map { |t| t.join }
-
-        pool.extend Module.new {
-          attr_accessor :checkins
-          def checkin conn
-            @checkins << conn
-            conn.object_id
-          end
-        }
-        pool.checkins = []
-
-        cleared_threads = pool.clear_stale_cached_connections!
-        assert((cleared_threads - threads.map { |x| x.object_id }).empty?,
-               "threads should have been removed")
-        assert_equal pool.checkins.length, threads.length
-      end
-
       def test_checkout_behaviour
         pool = ConnectionPool.new ActiveRecord::Base.connection_pool.spec
         connection = pool.connection
@@ -70,12 +46,15 @@ def test_checkout_behaviour
             assert thread_ids.include?(t.object_id)
           end
 
-          pool.connection
+          assert_deprecated do
+            pool.connection
+          end
           threads.each do |t|
             thread_ids = pool.instance_variable_get(:@reserved_connections).keys
             assert !thread_ids.include?(t.object_id)
           end
-        end.join()
+          pool.connection.close
+        end.join
 
       end
 
diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb
index 110a18772f..203dd054f1 100644
--- a/activerecord/test/cases/transactions_test.rb
+++ b/activerecord/test/cases/transactions_test.rb
@@ -563,6 +563,7 @@ def test_transaction_per_thread
               topic.approved = !topic.approved?
               topic.save!
             end
+            Topic.connection.close
           end
         end
 
@@ -598,6 +599,7 @@ def test_transaction_isolation__read_committed
               dev = Developer.find(1)
               assert_equal original_salary, dev.salary
             end
+            Developer.connection.close
           end
         end
 
@@ -610,6 +612,7 @@ def test_transaction_isolation__read_committed
               assert_equal original_salary, Developer.find(1).salary
             end
           end
+          Developer.connection.close
         end
 
         threads.each { |t| t.join }
-- 
GitLab