From 5f2f47fdfcc063a60ad1c3688e0c6d44b066d549 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Wed, 9 Apr 2014 22:41:33 +0200 Subject: [PATCH] Fixed #21553 -- Ensured unusable database connections get closed. --- django/db/backends/__init__.py | 7 ++++- django/db/backends/mysql/base.py | 2 +- django/db/backends/oracle/base.py | 2 +- .../db/backends/postgresql_psycopg2/base.py | 2 +- tests/backends/tests.py | 31 +++++++++++++++++++ 5 files changed, 40 insertions(+), 4 deletions(-) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index a8e0cc526c..4a03ccb658 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -351,9 +351,14 @@ class BaseDatabaseWrapper(object): def is_usable(self): """ Tests if the database connection is usable. + This function may assume that self.connection is not None. + + Actual implementations should take care not to raise exceptions + as that may prevent Django from recycling unusable connections. """ - raise NotImplementedError('subclasses of BaseDatabaseWrapper may require an is_usable() method') + raise NotImplementedError( + "subclasses of BaseDatabaseWrapper may require an is_usable() method") def close_if_unusable_or_obsolete(self): """ diff --git a/django/db/backends/mysql/base.py b/django/db/backends/mysql/base.py index 61a8ab72fc..cbe37fc84c 100644 --- a/django/db/backends/mysql/base.py +++ b/django/db/backends/mysql/base.py @@ -552,7 +552,7 @@ class DatabaseWrapper(BaseDatabaseWrapper): def is_usable(self): try: self.connection.ping() - except DatabaseError: + except Database.Error: return False else: return True diff --git a/django/db/backends/oracle/base.py b/django/db/backends/oracle/base.py index 830a8a9862..da0fdb121f 100644 --- a/django/db/backends/oracle/base.py +++ b/django/db/backends/oracle/base.py @@ -704,7 +704,7 @@ class DatabaseWrapper(BaseDatabaseWrapper): else: # Use a cx_Oracle cursor directly, bypassing Django's utilities. self.connection.cursor().execute("SELECT 1 FROM DUAL") - except DatabaseError: + except Database.Error: return False else: return True diff --git a/django/db/backends/postgresql_psycopg2/base.py b/django/db/backends/postgresql_psycopg2/base.py index dbba3d6d7d..47bcf2f41a 100644 --- a/django/db/backends/postgresql_psycopg2/base.py +++ b/django/db/backends/postgresql_psycopg2/base.py @@ -210,7 +210,7 @@ class DatabaseWrapper(BaseDatabaseWrapper): try: # Use a psycopg cursor directly, bypassing Django's utilities. self.connection.cursor().execute("SELECT 1") - except DatabaseError: + except Database.Error: return False else: return True diff --git a/tests/backends/tests.py b/tests/backends/tests.py index ee1e56676f..da851da166 100644 --- a/tests/backends/tests.py +++ b/tests/backends/tests.py @@ -665,6 +665,37 @@ class BackendTestCase(TestCase): self.assertTrue(cursor.closed) +class IsUsableTests(TransactionTestCase): + # Avoid using a regular TestCase because Django really dislikes closing + # the database connection inside a transaction at this point (#21202). + + available_apps = [] + + # Unfortunately with sqlite3 the in-memory test database cannot be closed. + @skipUnlessDBFeature('test_db_allows_multiple_connections') + def test_is_usable_after_database_disconnects(self): + """ + Test that is_usable() doesn't crash when the database disconnects. + + Regression for #21553. + """ + # Open a connection to the database. + with connection.cursor(): + pass + # Emulate a connection close by the database. + connection._close() + # Even then is_usable() should not raise an exception. + try: + self.assertFalse(connection.is_usable()) + finally: + # Clean up the mess created by connection._close(). Since the + # connection is already closed, this crashes on some backends. + try: + connection.close() + except Exception: + pass + + # We don't make these tests conditional because that means we would need to # check and differentiate between: # * MySQL+InnoDB, MySQL+MYISAM (something we currently can't do).