From ae67f353d262e8cc0d9706fe3cf8bbeea3c88a83 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 6 Mar 2014 17:49:24 +0000 Subject: [PATCH 1/7] Allow get_transaction_status on closed connections It's a local operation and the libpq functions has a NULL guard. Conflicts: NEWS --- NEWS | 2 ++ psycopg/connection_type.c | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 9f041727..c834d78d 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,8 @@ What's new in psycopg 2.4.7 - Work around `pip issue #1630 `__ making installation via ``pip -e git+url`` impossible (:ticket:`#18`). + - It is now possible to call `get_transaction_status()` on closed + connections. - Properly cleanup memory of broken connections (ticket #142). - Fixed build on Solaris 10 and 11 where the round() function is already declared (:ticket:`#146`). diff --git a/psycopg/connection_type.c b/psycopg/connection_type.c index 368c907f..90b3af79 100644 --- a/psycopg/connection_type.c +++ b/psycopg/connection_type.c @@ -647,8 +647,6 @@ psyco_conn_set_client_encoding(connectionObject *self, PyObject *args) static PyObject * psyco_conn_get_transaction_status(connectionObject *self, PyObject *args) { - EXC_IF_CONN_CLOSED(self); - return PyInt_FromLong((long)PQtransactionStatus(self->pgconn)); } From c8ee04a9fa30809fb39ebdbc3fb49bde066ff2a4 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 6 Mar 2014 18:11:17 +0000 Subject: [PATCH 2/7] Close a connection if PQexec returned NULL This happens for Socket connections, not for TCP ones, where a result containing an error is returned and correctly handled by pq_raise() Closes ticket #196 but not #192: poll() still doesn't change the connection closed. Conflicts: NEWS psycopg/pqpath.c --- NEWS | 2 ++ psycopg/pqpath.c | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index c834d78d..52cbe225 100644 --- a/NEWS +++ b/NEWS @@ -22,6 +22,8 @@ What's new in psycopg 2.4.7 failing to connect (from :ticket:`#192` discussion). - Don't segfault using poorly defined cursor subclasses which forgot to call the superclass init (:ticket:`#195`). + - Mark the connection closed when a Socket connection is broken, as it + happens for TCP connections instead (:ticket:`#196`). - Fixed overflow opening a lobject with an oid not fitting in a signed int (:ticket:`#203`). - Fixed possible segfault in named cursors creation. diff --git a/psycopg/pqpath.c b/psycopg/pqpath.c index 7290ad33..76032ec8 100644 --- a/psycopg/pqpath.c +++ b/psycopg/pqpath.c @@ -388,12 +388,20 @@ pq_complete_error(connectionObject *conn, PGresult **pgres, char **error) { Dprintf("pq_complete_error: pgconn = %p, pgres = %p, error = %s", conn->pgconn, *pgres, *error ? *error : "(null)"); - if (*pgres != NULL) + if (*pgres != NULL) { pq_raise(conn, NULL, *pgres); - else if (*error != NULL) { - PyErr_SetString(OperationalError, *error); } else { - PyErr_SetString(OperationalError, "unknown error"); + if (*error != NULL) { + PyErr_SetString(OperationalError, *error); + } else { + PyErr_SetString(OperationalError, "unknown error"); + } + /* Trivia: with a broken socket connection PQexec returns NULL, so we + * end up here. With a TCP connection we get a pgres with an error + * instead, and the connection gets closed in the pq_raise call above + * (see ticket #196) + */ + conn->closed = 2; } IFCLEARPGRES(*pgres); if (*error) { From bbe5a2597ca4978be77b4897f2c08b741225f613 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 5 Apr 2014 15:10:15 +0100 Subject: [PATCH 3/7] Don't set an exception witout GIL closing lobjects with a bad conn We ended up in this branch only for an excessively aggressive closing of the transaction that now I'm going to fix. --- psycopg/lobject_int.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/psycopg/lobject_int.c b/psycopg/lobject_int.c index 2f78e90b..43f2b179 100644 --- a/psycopg/lobject_int.c +++ b/psycopg/lobject_int.c @@ -253,7 +253,7 @@ lobject_close_locked(lobjectObject *self, char **error) return 0; break; default: - PyErr_SetString(OperationalError, "the connection is broken"); + *error = strdup("the connection is broken"); return -1; break; } From ddf97a0cdc0d692d87bd8b83cc56fdb72e1fe693 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 5 Apr 2014 15:11:43 +0100 Subject: [PATCH 4/7] Fixed attempt of closing an already closed lobject on dealloc This results in a "null without exception set" in the corrent state, which is caused by the connection being unexpectedly closed anyway. --- psycopg/lobject_type.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/psycopg/lobject_type.c b/psycopg/lobject_type.c index 4a47721f..fad48a20 100644 --- a/psycopg/lobject_type.c +++ b/psycopg/lobject_type.c @@ -359,7 +359,7 @@ lobject_dealloc(PyObject* obj) { lobjectObject *self = (lobjectObject *)obj; - if (self->conn) { /* if not, init failed */ + if (self->conn && self->fd != -1) { if (lobject_close(self) < 0) PyErr_Print(); Py_XDECREF((PyObject*)self->conn); From cdb206d3e7a3d11ff072e1b2a1073b8f3407664b Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 5 Apr 2014 15:14:00 +0100 Subject: [PATCH 5/7] Check the connection is really bad on exception before closing it We end up here without a pgres sometimes (e.g. from lobject errors) --- psycopg/pqpath.c | 4 +++- tests/test_lobject.py | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/psycopg/pqpath.c b/psycopg/pqpath.c index 76032ec8..65b226e1 100644 --- a/psycopg/pqpath.c +++ b/psycopg/pqpath.c @@ -401,7 +401,9 @@ pq_complete_error(connectionObject *conn, PGresult **pgres, char **error) * instead, and the connection gets closed in the pq_raise call above * (see ticket #196) */ - conn->closed = 2; + if (CONNECTION_BAD == PQstatus(conn->pgconn)) { + conn->closed = 2; + } } IFCLEARPGRES(*pgres); if (*error) { diff --git a/tests/test_lobject.py b/tests/test_lobject.py index 597e07c3..01607aab 100755 --- a/tests/test_lobject.py +++ b/tests/test_lobject.py @@ -138,6 +138,7 @@ class LargeObjectTests(LargeObjectMixin, unittest.TestCase): self.assertRaises(psycopg2.OperationalError, self.conn.lobject, 0, "w", lo.oid) + self.assert_(not self.conn.closed) def test_import(self): self.tmpdir = tempfile.mkdtemp() From 71eeb9086a36ca98814c4292c10dd89244677e90 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 5 Apr 2014 15:32:16 +0100 Subject: [PATCH 6/7] Close the connection if discovered bad on poll() Conflicts: NEWS --- NEWS | 1 + psycopg/pqpath.c | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/NEWS b/NEWS index 52cbe225..4e0c029d 100644 --- a/NEWS +++ b/NEWS @@ -26,6 +26,7 @@ What's new in psycopg 2.4.7 happens for TCP connections instead (:ticket:`#196`). - Fixed overflow opening a lobject with an oid not fitting in a signed int (:ticket:`#203`). + - Mark the connection closed if found broken on `poll()`. - Fixed possible segfault in named cursors creation. - Fixed debug build on Windows, thanks to James Emerton. diff --git a/psycopg/pqpath.c b/psycopg/pqpath.c index 65b226e1..17f7a972 100644 --- a/psycopg/pqpath.c +++ b/psycopg/pqpath.c @@ -774,6 +774,12 @@ pq_is_busy(connectionObject *conn) Dprintf("pq_is_busy: PQconsumeInput() failed"); pthread_mutex_unlock(&(conn->lock)); Py_BLOCK_THREADS; + + /* if the libpq says pgconn is lost, close the py conn */ + if (CONNECTION_BAD == PQstatus(conn->pgconn)) { + conn->closed = 2; + } + PyErr_SetString(OperationalError, PQerrorMessage(conn->pgconn)); return -1; } @@ -803,6 +809,12 @@ pq_is_busy_locked(connectionObject *conn) if (PQconsumeInput(conn->pgconn) == 0) { Dprintf("pq_is_busy_locked: PQconsumeInput() failed"); + + /* if the libpq says pgconn is lost, close the py conn */ + if (CONNECTION_BAD == PQstatus(conn->pgconn)) { + conn->closed = 2; + } + PyErr_SetString(OperationalError, PQerrorMessage(conn->pgconn)); return -1; } From e53f63b2d68a009a4138d6a499126b27908ac8fd Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 5 Apr 2014 13:37:42 +0100 Subject: [PATCH 7/7] Don't specify 0 or 1 in closed docs There's also 2 which means broken. But I prefer to leave that as implementation detail. --- doc/src/connection.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/src/connection.rst b/doc/src/connection.rst index a82d4e7f..93d65ebd 100644 --- a/doc/src/connection.rst +++ b/doc/src/connection.rst @@ -283,8 +283,8 @@ The ``connection`` class .. attribute:: closed - Read-only attribute reporting whether the database connection is open - (0) or closed (1). + Read-only integer attribute: 0 if the connection is open, nonzero if + it is closed or broken. .. method:: cancel