Do away with pool._refs

This collection was added only for the benefit of unit tests
and is unnecessary for the pool to function.  As SQLAlchemy 2.0
will be removing the automatic handling of connections that are
garbage collection, remove this collection so that we ultimately
don't need a weakref handler to do anything within the pool.
The handler will do nothing other than emit a warning that
a connection was dereferenced without being explicitly returned
to the pool, invalidated, or detached.

Change-Id: I4ca196270d5714efbac44dbf6f034e8c7f0af58a

diff --git a/lib/sqlalchemy/pool/__init__.py b/lib/sqlalchemy/pool/__init__.py
index 058d595c8bf795ddcffd41355b58140cf594549f..eb0d3751733e5b7c32ebeeba463393533d0481c1 100644 (file)
--- a/lib/sqlalchemy/pool/__init__.py
+++ b/lib/sqlalchemy/pool/__init__.py
@@ -21,7 +21,6 @@ from . import events  # noqa
 from .base import _ConnectionFairy  # noqa
 from .base import _ConnectionRecord  # noqa
 from .base import _finalize_fairy  # noqa
-from .base import _refs  # noqa
 from .base import Pool
 from .base import reset_commit
 from .base import reset_none

diff --git a/lib/sqlalchemy/pool/base.py b/lib/sqlalchemy/pool/base.py
index 508d036309691d52a741ffdffd2a9ab702e9ebd3..bdb981699ab5613084c68bbae75c6bc03ce9739a 100644 (file)
--- a/lib/sqlalchemy/pool/base.py
+++ b/lib/sqlalchemy/pool/base.py
@@ -425,7 +425,6 @@ class _ConnectionRecord(object):
             lambda ref: _finalize_fairy
             and _finalize_fairy(None, rec, pool, ref, echo),
         )
-        _refs.add(rec)
         if echo:
             pool.logger.debug(
                 "Connection %r checked out from pool", dbapi_connection
@@ -594,7 +593,6 @@ def _finalize_fairy(
     been garbage collected.
 
     """
-    _refs.discard(connection_record)
 
     if ref is not None:
         if connection_record.fairy_ref is not ref:
@@ -633,9 +631,6 @@ def _finalize_fairy(
         connection_record.checkin()
 
 
-_refs = set()
-
-
 class _ConnectionFairy(object):
 
     """Proxies a DBAPI connection and provides return-on-dereference
@@ -918,7 +913,6 @@ class _ConnectionFairy(object):
 
         if self._connection_record is not None:
             rec = self._connection_record
-            _refs.remove(rec)
             rec.fairy_ref = None
             rec.connection = None
             # TODO: should this be _return_conn?

diff --git a/lib/sqlalchemy/testing/assertions.py b/lib/sqlalchemy/testing/assertions.py
index a465bedd4916977ab93aa8b29e9b4699f6879f49..c74259bdf8e4558753f46f8010abdcd1209076b3 100644 (file)
--- a/lib/sqlalchemy/testing/assertions.py
+++ b/lib/sqlalchemy/testing/assertions.py
@@ -13,13 +13,12 @@ import warnings
 
 from . import assertsql
 from . import config
+from . import engines
 from . import mock
-from . import util as testutil
 from .exclusions import db_spec
 from .util import fail
 from .. import exc as sa_exc
 from .. import orm
-from .. import pool
 from .. import schema
 from .. import types as sqltypes
 from .. import util
@@ -183,49 +182,8 @@ def global_cleanup_assertions():
     _assert_no_stray_pool_connections()
 
 
-_STRAY_CONNECTION_FAILURES = 0
-
-
 def _assert_no_stray_pool_connections():
-    global _STRAY_CONNECTION_FAILURES
-
-    # lazy gc on cPython means "do nothing."  pool connections
-    # shouldn't be in cycles, should go away.
-    testutil.lazy_gc()
-
-    # however, once in awhile, on an EC2 machine usually,
-    # there's a ref in there.  usually just one.
-    if pool._refs:
-
-        # OK, let's be somewhat forgiving.
-        _STRAY_CONNECTION_FAILURES += 1
-
-        print(
-            "Encountered a stray connection in test cleanup: %s"
-            % str(pool._refs)
-        )
-        # then do a real GC sweep.   We shouldn't even be here
-        # so a single sweep should really be doing it, otherwise
-        # there's probably a real unreachable cycle somewhere.
-        testutil.gc_collect()
-
-    # if we've already had two of these occurrences, or
-    # after a hard gc sweep we still have pool._refs?!
-    # now we have to raise.
-    if pool._refs:
-        err = str(pool._refs)
-
-        # but clean out the pool refs collection directly,
-        # reset the counter,
-        # so the error doesn't at least keep happening.
-        pool._refs.clear()
-        _STRAY_CONNECTION_FAILURES = 0
-        warnings.warn(
-            "Stray connection refused to leave " "after gc.collect(): %s" % err
-        )
-    elif _STRAY_CONNECTION_FAILURES > 10:
-        assert False, "Encountered more than 10 stray connections"
-        _STRAY_CONNECTION_FAILURES = 0
+    engines.testing_reaper.assert_all_closed()
 
 
 def eq_regex(a, b, msg=None):

diff --git a/lib/sqlalchemy/testing/engines.py b/lib/sqlalchemy/testing/engines.py
index ff4f89606edea4d9d02e57ff4fec9a2c6f676611..4f413915a5dfa65e07952c7706ef9a0991b51cc5 100644 (file)
--- a/lib/sqlalchemy/testing/engines.py
+++ b/lib/sqlalchemy/testing/engines.py
@@ -12,7 +12,6 @@ import warnings
 import weakref
 
 from . import config
-from . import uses_deprecated
 from .util import decorator
 from .. import event
 from .. import pool
@@ -24,7 +23,13 @@ class ConnectionKiller(object):
         self.testing_engines = weakref.WeakKeyDictionary()
         self.conns = set()
 
+    def add_pool(self, pool):
+        event.listen(pool, "connect", self.connect)
+        event.listen(pool, "checkout", self.checkout)
+        event.listen(pool, "invalidate", self.invalidate)
+
     def add_engine(self, engine):
+        self.add_pool(engine.pool)
         self.testing_engines[engine] = True
 
     def connect(self, dbapi_conn, con_record):
@@ -75,7 +80,6 @@ class ConnectionKiller(object):
         else:
             self._stop_test_ctx_aggressive()
 
-    @uses_deprecated()
     def _stop_test_ctx_minimal(self):
         self.close_all()
 
@@ -85,7 +89,6 @@ class ConnectionKiller(object):
             if rec is not config.db:
                 rec.dispose()
 
-    @uses_deprecated()
     def _stop_test_ctx_aggressive(self):
         self.close_all()
         for conn, rec in list(self.conns):
@@ -265,9 +268,6 @@ def testing_engine(url=None, options=None):
         engine.pool._timeout = 0
         engine.pool._max_overflow = 0
     if use_reaper:
-        event.listen(engine.pool, "connect", testing_reaper.connect)
-        event.listen(engine.pool, "checkout", testing_reaper.checkout)
-        event.listen(engine.pool, "invalidate", testing_reaper.invalidate)
         testing_reaper.add_engine(engine)
 
     return engine

diff --git a/test/aaa_profiling/test_pool.py b/test/aaa_profiling/test_pool.py
index d74dcf62774fcb0aa1c7ca1e24013e350f4207e1..538eee45d41c90c46122eb6a0facd76c28ac3228 100644 (file)
--- a/test/aaa_profiling/test_pool.py
+++ b/test/aaa_profiling/test_pool.py
@@ -1,4 +1,3 @@
-from sqlalchemy import pool as pool_module
 from sqlalchemy.pool import QueuePool
 from sqlalchemy.testing import AssertsExecutionResults
 from sqlalchemy.testing import fixtures
@@ -18,13 +17,6 @@ class QueuePoolTest(fixtures.TestBase, AssertsExecutionResults):
         def close(self):
             pass
 
-    def teardown(self):
-        # the tests leave some fake connections
-        # around which don't necessarily
-        # get gc'ed as quickly as we'd like all the time,
-        # particularly for non-refcount gc
-        pool_module._refs.clear()
-
     def setup(self):
         # create a throwaway pool which
         # has the effect of initializing

diff --git a/test/engine/test_pool.py b/test/engine/test_pool.py
index e4d6dba5736866e8af1319d59e2ea2e416565963..cfe20f5ec07a6891309538290c25d44eece90a61 100644 (file)
--- a/test/engine/test_pool.py
+++ b/test/engine/test_pool.py
@@ -725,6 +725,8 @@ class QueuePoolTest(PoolTestBase):
 
     def _do_testqueuepool(self, useclose=False):
         p = self._queuepool_fixture(pool_size=3, max_overflow=-1)
+        reaper = testing.engines.ConnectionKiller()
+        reaper.add_pool(p)
 
         def status(pool):
             return (
@@ -772,8 +774,8 @@ class QueuePoolTest(PoolTestBase):
             lazy_gc()
         self.assert_(status(p) == (3, 2, 0, 1))
         c1.close()
-        lazy_gc()
-        assert not pool._refs
+
+        reaper.assert_all_closed()
 
     def test_timeout_accessor(self):
         expected_timeout = 123
@@ -837,7 +839,7 @@ class QueuePoolTest(PoolTestBase):
             assert t < 14, "Not all timeouts were < 14 seconds %r" % timeouts
 
     def _test_overflow(self, thread_count, max_overflow):
-        gc_collect()
+        reaper = testing.engines.ConnectionKiller()
 
         dbapi = MockDBAPI()
         mutex = threading.Lock()
@@ -850,6 +852,7 @@ class QueuePoolTest(PoolTestBase):
         p = pool.QueuePool(
             creator=creator, pool_size=3, timeout=2, max_overflow=max_overflow
         )
+        reaper.add_pool(p)
         peaks = []
 
         def whammy():
@@ -873,8 +876,7 @@ class QueuePoolTest(PoolTestBase):
 
         self.assert_(max(peaks) <= max_overflow)
 
-        lazy_gc()
-        assert not pool._refs
+        reaper.assert_all_closed()
 
     def test_overflow_reset_on_failed_connect(self):
         dbapi = Mock()