Restore use_threadlocal equivalent behavior to SingletonThreadPool

Fixed behavioral regression as a result of deprecating the "use_threadlocal"
flag for :class:`.Pool`, where the :class:`.SingletonThreadPool` no longer
makes use of this option which causes the "rollback on return" logic to take
place when the same :class:`.Engine` is used multiple times in the context
of a transaction to connect or implicitly execute, thereby cancelling the
transaction.   While this is not the recommended way to work with engines
and connections, it is nonetheless a confusing behavioral change as when
using :class:`.SingletonThreadPool`, the transaction should stay open
regardless of what else is done with the same engine in the same thread.
The ``use_threadlocal`` flag remains deprecated however the
:class:`.SingletonThreadPool` now implements its own version of the same
logic.

Fixes: #4585
Change-Id: I906293f2d0a5d14ed46cd9e64305a6481505a5a3

diff --git a/doc/build/changelog/unreleased_13/4585.rst b/doc/build/changelog/unreleased_13/4585.rst
new file mode 100644 (file)
index 0000000..f9e13ca
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4585.rst
@@ -0,0 +1,17 @@
+.. change::
+   :tags: bug, pool
+   :tickets: 4585
+
+   Fixed behavioral regression as a result of deprecating the "use_threadlocal"
+   flag for :class:`.Pool`, where the :class:`.SingletonThreadPool` no longer
+   makes use of this option which causes the "rollback on return" logic to take
+   place when the same :class:`.Engine` is used multiple times in the context
+   of a transaction to connect or implicitly execute, thereby cancelling the
+   transaction.   While this is not the recommended way to work with engines
+   and connections, it is nonetheless a confusing behavioral change as when
+   using :class:`.SingletonThreadPool`, the transaction should stay open
+   regardless of what else is done with the same engine in the same thread.
+   The ``use_threadlocal`` flag remains deprecated however the
+   :class:`.SingletonThreadPool` now implements its own version of the same
+   logic.
+

diff --git a/lib/sqlalchemy/pool/impl.py b/lib/sqlalchemy/pool/impl.py
index ada3196612cab21aefd4955889b3363f2172ce87..e1a457bf3e6a58718bdb6a170ebb6e762988befb 100644 (file)
--- a/lib/sqlalchemy/pool/impl.py
+++ b/lib/sqlalchemy/pool/impl.py
@@ -13,6 +13,7 @@
 import traceback
 import weakref
 
+from .base import _ConnectionFairy
 from .base import _ConnectionRecord
 from .base import Pool
 from .. import exc
@@ -288,6 +289,7 @@ class SingletonThreadPool(Pool):
     def __init__(self, creator, pool_size=5, **kw):
         Pool.__init__(self, creator, **kw)
         self._conn = threading.local()
+        self._fairy = threading.local()
         self._all_conns = set()
         self.size = pool_size
 
@@ -346,6 +348,25 @@ class SingletonThreadPool(Pool):
         self._all_conns.add(c)
         return c
 
+    def connect(self):
+        # vendored from Pool to include use_threadlocal behavior
+        try:
+            rec = self._fairy.current()
+        except AttributeError:
+            pass
+        else:
+            if rec is not None:
+                return rec._checkout_existing()
+
+        return _ConnectionFairy._checkout(self, self._fairy)
+
+    def _return_conn(self, record):
+        try:
+            del self._fairy.current
+        except AttributeError:
+            pass
+        self._do_return_conn(record)
+
 
 class StaticPool(Pool):
 

diff --git a/test/engine/test_pool.py b/test/engine/test_pool.py
index 3722ce9b9fdd46b1c3bf2a9e4ae6891bf46f4081..6cd2e457281e3647b07ddbd49f4caa9e759d4d3f 100644 (file)
--- a/test/engine/test_pool.py
+++ b/test/engine/test_pool.py
@@ -1818,6 +1818,31 @@ class SingletonThreadPoolTest(PoolTestBase):
             still_opened = len([c for c in sr if not c.close.call_count])
             eq_(still_opened, 3)
 
+    def test_no_rollback_from_nested_connections(self):
+        dbapi = MockDBAPI()
+
+        lock = threading.Lock()
+
+        def creator():
+            # the mock iterator isn't threadsafe...
+            with lock:
+                return dbapi.connect()
+
+        p = pool.SingletonThreadPool(creator=creator, pool_size=3)
+
+        c1 = p.connect()
+        mock_conn = c1.connection
+
+        c2 = p.connect()
+        is_(c1, c2)
+
+        c2.close()
+
+        eq_(mock_conn.mock_calls, [])
+        c1.close()
+
+        eq_(mock_conn.mock_calls, [call.rollback()])
+
 
 class AssertionPoolTest(PoolTestBase):
     def test_connect_error(self):