--- /dev/null
+.. change::
+ :tags: bug, engine
+ :tickets: 4252
+ :versions: 1.3.0b1
+
+ Fixed connection pool issue whereby if a disconnection error were raised
+ during the connection pool's "reset on return" sequence in conjunction with
+ an explicit transaction opened against the enclosing :class:`.Connection`
+ object (such as from calling :meth:`.Session.close` without a rollback or
+ commit, or calling :meth:`.Connection.close` without first closing a
+ transaction declared with :meth:`.Connection.begin`), a double-checkin would
+ result, which could then lead towards concurrent checkouts of the same
+ connection. The double-checkin condition is now prevented overall by an
+ assertion, as well as the specific double-checkin scenario has been
+ fixed.
\ No newline at end of file
def _checkin_failed(self, err):
self.invalidate(e=err)
- self.checkin()
+ self.checkin(_no_fairy_ref=True)
- def checkin(self):
+ def checkin(self, _no_fairy_ref=False):
+ if self.fairy_ref is None and not _no_fairy_ref:
+ util.warn("Double checkin attempted on %s" % self)
+ return
self.fairy_ref = None
connection = self.connection
pool = self.__pool
if not isinstance(e, Exception):
raise
- if connection_record:
+ if connection_record and connection_record.fairy_ref is not None:
connection_record.checkin()
from sqlalchemy.testing import eq_, assert_raises, is_not_, is_
from sqlalchemy.testing.engines import testing_engine
from sqlalchemy.testing import fixtures
+from sqlalchemy.testing import assert_raises_message
import random
from sqlalchemy.testing.mock import Mock, call, patch, ANY
import weakref
c2 = p.connect()
assert c2.connection is not None
+ def test_no_double_checkin(self):
+ p = self._queuepool_fixture(pool_size=1)
+
+ c1 = p.connect()
+ rec = c1._connection_record
+ c1.close()
+ assert_raises_message(
+ Warning,
+ "Double checkin attempted on %s" % rec,
+ rec.checkin
+ )
+
class ResetOnReturnTest(PoolTestBase):
def _fixture(self, **kw):
assert not dbapi.connect().rollback.called
assert dbapi.connect().commit.called
+ def test_reset_agent_disconnect(self):
+ dbapi, p = self._fixture(reset_on_return='rollback')
+
+ class Agent(object):
+ def __init__(self, conn):
+ self.conn = conn
+
+ def rollback(self):
+ p._invalidate(self.conn)
+ raise Exception("hi")
+
+ def commit(self):
+ self.conn.commit()
+
+ c1 = p.connect()
+ c1._reset_agent = Agent(c1)
+ c1.close()
+
+ # no warning raised. We know it would warn due to
+ # QueuePoolTest.test_no_double_checkin
+
class SingletonThreadPoolTest(PoolTestBase):
@testing.requires.threading_with_mock