From 7c5afd6977c2d71a1a3f9550849580b75851a65b Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 17 Mar 2019 23:06:55 +0000 Subject: [PATCH 1/4] Added test to reproduce ticket #829 Unrelated processes close the FD of the connection. This happens in Python 3.6 but not 2.7. Let's see if travis shows where else it fails... --- tests/test_connection.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/test_connection.py b/tests/test_connection.py index 77f3b29d..75abcc02 100755 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -357,6 +357,34 @@ class ConnectionTests(ConnectingTestCase): conn.close() self.assert_(conn.pgconn_ptr is None) + @slow + def test_multiprocess_close(self): + script = ("""\ +import time +import threading +import multiprocessing +import psycopg2 + +def thread(): + conn = psycopg2.connect(%(dsn)r) + curs = conn.cursor() + for i in range(10): + curs.execute("select 1") + time.sleep(0.1) + +def process(): + time.sleep(0.2) + +t = threading.Thread(target=thread, name='mythread') +t.start() +time.sleep(0.2) +multiprocessing.Process(target=process, name='myprocess').start() +t.join() +""" % {'dsn': dsn}) + + out = sp.check_output([sys.executable, '-c', script], stderr=sp.STDOUT) + self.assertEqual(out, b'', out.decode('ascii')) + class ParseDsnTestCase(ConnectingTestCase): def test_parse_dsn(self): From 62a078fe0cae5741cdf49c6ab5431083139f5253 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 17 Mar 2019 23:55:04 +0000 Subject: [PATCH 2/4] subprocess test function moved into a module It won't work on windows if it's in the script: failing with errors such as: AttributeError: 'module' object has no attribute 'process' or: Can't get attribute 'process' on --- tests/test_connection.py | 43 +++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/tests/test_connection.py b/tests/test_connection.py index 75abcc02..09b2d9a0 100755 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -22,14 +22,16 @@ # FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public # License for more details. -import ctypes import gc import os import re -import subprocess as sp import sys -import threading import time +import ctypes +import shutil +import tempfile +import threading +import subprocess as sp from collections import deque from operator import attrgetter from weakref import ref @@ -359,10 +361,11 @@ class ConnectionTests(ConnectingTestCase): @slow def test_multiprocess_close(self): - script = ("""\ + dir = tempfile.mkdtemp() + try: + with open(os.path.join(dir, "mptest.py"), 'w') as f: + f.write("""\ import time -import threading -import multiprocessing import psycopg2 def thread(): @@ -374,16 +377,28 @@ def thread(): def process(): time.sleep(0.2) - -t = threading.Thread(target=thread, name='mythread') -t.start() -time.sleep(0.2) -multiprocessing.Process(target=process, name='myprocess').start() -t.join() """ % {'dsn': dsn}) - out = sp.check_output([sys.executable, '-c', script], stderr=sp.STDOUT) - self.assertEqual(out, b'', out.decode('ascii')) + script = ("""\ +import sys +sys.path.insert(0, %(dir)r) +import time +import threading +import multiprocessing +import mptest + +t = threading.Thread(target=mptest.thread, name='mythread') +t.start() +time.sleep(0.2) +multiprocessing.Process(target=mptest.process, name='myprocess').start() +t.join() +""" % {'dir': dir}) + + out = sp.check_output( + [sys.executable, '-c', script], stderr=sp.STDOUT) + self.assertEqual(out, b'', out) + finally: + shutil.rmtree(dir, ignore_errors=True) class ParseDsnTestCase(ConnectingTestCase): From 17b0c6133800163bb264e9553ddf03f8d28f50e1 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Mon, 18 Mar 2019 00:27:16 +0000 Subject: [PATCH 3/4] Don't close connections from forked processes On Py3 subprocessing will actually GC the objects and the FD is open, resulting in connections closed in different processes. The behaviour is verified in py 3.4 to 3.7 at least, --- psycopg/config.h | 14 ++++++++++++-- psycopg/connection.h | 3 +++ psycopg/connection_type.c | 14 +++++++++++++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/psycopg/config.h b/psycopg/config.h index c9a7f892..4fba0036 100644 --- a/psycopg/config.h +++ b/psycopg/config.h @@ -33,6 +33,18 @@ # define HIDDEN #endif +/* support for getpid() */ +#if defined( __GNUC__) +#define CONN_CHECK_PID +#include +#include +#endif +#ifdef _WIN32 +/* Windows doesn't seem affected by bug #829: just make it compile. */ +#define pid_t int +#endif + + /* debug printf-like function */ #ifdef PSYCOPG_DEBUG extern HIDDEN int psycopg_debug_enabled; @@ -40,8 +52,6 @@ extern HIDDEN int psycopg_debug_enabled; #if defined( __GNUC__) && !defined(__APPLE__) #ifdef PSYCOPG_DEBUG -#include -#include #define Dprintf(fmt, args...) \ if (!psycopg_debug_enabled) ; else \ fprintf(stderr, "[%d] " fmt "\n", (int) getpid() , ## args) diff --git a/psycopg/connection.h b/psycopg/connection.h index bab8bf84..acf4e451 100644 --- a/psycopg/connection.h +++ b/psycopg/connection.h @@ -141,6 +141,9 @@ struct connectionObject { int isolevel; int readonly; int deferrable; + + /* the pid this connection was created into */ + pid_t procpid; }; /* map isolation level values into a numeric const */ diff --git a/psycopg/connection_type.c b/psycopg/connection_type.c index f4d650b5..19cd733b 100644 --- a/psycopg/connection_type.c +++ b/psycopg/connection_type.c @@ -1367,6 +1367,10 @@ connection_setup(connectionObject *self, const char *dsn, long int async) self->isolevel = ISOLATION_LEVEL_DEFAULT; self->readonly = STATE_DEFAULT; self->deferrable = STATE_DEFAULT; +#ifdef CONN_CHECK_PID + self->procpid = getpid(); +#endif + /* other fields have been zeroed by tp_alloc */ pthread_mutex_init(&(self->lock), NULL); @@ -1420,7 +1424,15 @@ connection_dealloc(PyObject* obj) * resulting in a double-free segfault (ticket #166). */ PyObject_GC_UnTrack(self); - conn_close(self); + /* close the connection only if this is the same process it was created + * into, otherwise using multiprocessing we may close the connection + * belonging to another process. */ +#ifdef CONN_CHECK_PID + if (self->procpid == getpid()) +#endif + { + conn_close(self); + } if (self->weakreflist) { PyObject_ClearWeakRefs(obj); From f8f5a77838d0d356d18552d2fbb295335cad7de3 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Mon, 18 Mar 2019 01:51:29 +0000 Subject: [PATCH 4/4] Mention closed #829 in NEWS file --- NEWS | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 7dc3e1b8..c56abfcd 100644 --- a/NEWS +++ b/NEWS @@ -24,8 +24,6 @@ New features: (:ticket:`#732`). - Added *fetch* parameter to `~psycopg2.extras.execute_values()` function (:ticket:`#813`). -- Fixed adaptation of numeric subclasses such as `~enum.IntEnum` - (:ticket:`#591`). - `!str()` on `~psycopg2.extras.Range` produces a human-readable representation (:ticket:`#773`). - `~psycopg2.extras.DictCursor` and `~psycopg2.extras.RealDictCursor` rows @@ -33,8 +31,15 @@ New features: - Added `~psycopg2.extensions.Diagnostics.severity_nonlocalized` attribute on the `~psycopg2.extensions.Diagnostics` object (:ticket:`#783`). - More efficient `~psycopg2.extras.NamedTupleCursor` (:ticket:`#838`). -- Async communication improved to fix blocking on results returned at - different times (:ticket:`#856`). + +Bug fixes: + +- Fixed connections occasionally broken by the unrelated use of the + multiprocessing module (:ticket:`#829`). +- Fixed async communication blocking if results are returned in different + chunks, e.g. with notices interspersed to the results (:ticket:`#856`). +- Fixed adaptation of numeric subclasses such as `~enum.IntEnum` + (:ticket:`#591`). Other changes: