Emit a warning when a connection or cursor isn't closed

This follows the semantics of Python file objects. When the object is
garbage collected and it has not been closed, emit a ResourceWarning.
This allows library users to notice inconsistent and non-deterministic
resource management and fix it. Users should use context managers and
finally blocks to always close cursors and connections when they are not
longer required.

For example, in Python, not closing a file results in the following:

    $ python3 -Walways
    >>> f = open('foo', 'w')
    >>> del f
    __main__:1: ResourceWarning: unclosed file <_io.TextIOWrapper name='foo' mode='w' encoding='UTF-8'>
    ResourceWarning: Enable tracemalloc to get the object allocation traceback

psycopg now acts the same way:

    $ python3 -Walways
    >>> import psycopg2
    >>> c = psycopg2.connect(database='psycopg2_test')
    >>> del c
    <stdin>:1: ResourceWarning: unclosed connection <connection object at 0x7fc4aec38d50; dsn: 'dbname=psycopg2_test', closed: 0>
    ResourceWarning: Enable tracemalloc to get the object allocation traceback

All warnings noticed during testing has been fixed.

psycopg/connection_type.c

@@ -59,7 +59,7 @@ static PyObject *
 psyco_conn_cursor(connectionObject *self, PyObject *args, PyObject *kwargs)
 {
     PyObject *obj = NULL;
-    PyObject *rv = NULL;
+    cursorObject *curs = NULL;
     PyObject *name = Py_None;
     PyObject *factory = Py_None;
     PyObject *withhold = Py_False;
@@ -110,14 +110,17 @@ psyco_conn_cursor(connectionObject *self, PyObject *args, PyObject *kwargs)
     if (PyObject_IsInstance(obj, (PyObject *)&cursorType) == 0) {
         PyErr_SetString(PyExc_TypeError,
             "cursor factory must be subclass of psycopg2.extensions.cursor");
+        Py_DECREF(obj);
+        obj = NULL;
         goto exit;
     }
 
-    if (0 > curs_withhold_set((cursorObject *)obj, withhold)) {
-        goto exit;
+    curs = (cursorObject *)obj;
+    if (0 > curs_withhold_set(curs, withhold)) {
+        goto error;
     }
-    if (0 > curs_scrollable_set((cursorObject *)obj, scrollable)) {
-        goto exit;
+    if (0 > curs_scrollable_set(curs, scrollable)) {
+        goto error;
     }
 
     Dprintf("psyco_conn_cursor: new cursor at %p: refcnt = "
@@ -125,12 +128,26 @@ psyco_conn_cursor(connectionObject *self, PyObject *args, PyObject *kwargs)
         obj, Py_REFCNT(obj)
     );
 
-    rv = obj;
     obj = NULL;
+    goto exit;
+
+error:
+    {
+        PyObject *error_type, *error_value, *error_traceback;
+        PyObject *close;
+        curs = NULL;
+        PyErr_Fetch(&error_type, &error_value, &error_traceback);
+        close = PyObject_CallMethod(obj, "close", NULL);
+        if (close)
+            Py_DECREF(close);
+        else
+            PyErr_WriteUnraisable(obj);
+        PyErr_Restore(error_type, error_value, error_traceback);
+    }
 
 exit:
     Py_XDECREF(obj);
-    return rv;
+    return (PyObject *)curs;
 }
 
 
@@ -1366,6 +1383,9 @@ connection_dealloc(PyObject* obj)
 {
     connectionObject *self = (connectionObject *)obj;
 
+    if (PyObject_CallFinalizerFromDealloc(obj) < 0)
+        return;
+
     /* Make sure to untrack the connection before calling conn_close, which may
      * allow a different thread to try and dealloc the connection again,
      * resulting in a double-free segfault (ticket #166). */
@@ -1405,6 +1425,31 @@ connection_dealloc(PyObject* obj)
     Py_TYPE(obj)->tp_free(obj);
 }
 
+#if PY_3
+static void
+connection_finalize(PyObject *obj)
+{
+    connectionObject *self = (connectionObject *)obj;
+
+#ifdef CONN_CHECK_PID
+    if (self->procpid == getpid())
+#endif
+    {
+        if (!self->closed) {
+            PyObject *error_type, *error_value, *error_traceback;
+            /* Save the current exception, if any. */
+            PyErr_Fetch(&error_type, &error_value, &error_traceback);
+
+            if (PyErr_WarnFormat(PyExc_ResourceWarning, 1, "unclosed connection %R", obj))
+                PyErr_WriteUnraisable(obj);
+
+            /* Restore the saved exception. */
+            PyErr_Restore(error_type, error_value, error_traceback);
+        }
+    }
+}
+#endif
+
 static int
 connection_init(PyObject *obj, PyObject *args, PyObject *kwds)
 {
@@ -1479,7 +1524,7 @@ PyTypeObject connectionType = {
     0,          /*tp_setattro*/
     0,          /*tp_as_buffer*/
     Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC |
-        Py_TPFLAGS_HAVE_WEAKREFS,
+        Py_TPFLAGS_HAVE_WEAKREFS | Py_TPFLAGS_HAVE_FINALIZE,
                 /*tp_flags*/
     connectionType_doc, /*tp_doc*/
     (traverseproc)connection_traverse, /*tp_traverse*/
@@ -1499,4 +1544,16 @@ PyTypeObject connectionType = {
     connection_init, /*tp_init*/
     0,          /*tp_alloc*/
     connection_new, /*tp_new*/
+    0,                   /* tp_free */
+    0,                   /* tp_is_gc */
+    0,                   /* tp_bases */
+    0,                   /* tp_mro */
+    0,                   /* tp_cache */
+    0,                   /* tp_subclasses */
+    0,                   /* tp_weaklist */
+#if PY_3
+    0,                   /* tp_del */
+    0,                   /* tp_version_tag */
+    connection_finalize, /* tp_finalize */
+#endif
 };

psycopg/cursor_type.c

@@ -39,6 +39,7 @@
 
 #include <stdlib.h>
 
+#define cursor_closed(self) ((self)->closed || ((self)->conn && (self)->conn->closed))
 
 /** DBAPI methods **/
 
@@ -1637,7 +1638,7 @@ exit:
 static PyObject *
 curs_closed_get(cursorObject *self, void *closure)
 {
-    return PyBool_FromLong(self->closed || (self->conn && self->conn->closed));
+    return PyBool_FromLong(cursor_closed(self));
 }
 
 /* extension: withhold - get or set "WITH HOLD" for named cursors */
@@ -1945,6 +1946,9 @@ cursor_dealloc(PyObject* obj)
 {
     cursorObject *self = (cursorObject *)obj;
 
+    if (PyObject_CallFinalizerFromDealloc(obj) < 0)
+        return;
+
     PyObject_GC_UnTrack(self);
 
     if (self->weakreflist) {
@@ -1965,6 +1969,28 @@ cursor_dealloc(PyObject* obj)
     Py_TYPE(obj)->tp_free(obj);
 }
 
+#if PY_3
+static void
+cursor_finalize(PyObject *obj)
+{
+    cursorObject *self = (cursorObject *)obj;
+
+    if (!cursor_closed(self)) {
+        PyObject *error_type, *error_value, *error_traceback;
+        /* Save the current exception, if any. */
+        PyErr_Fetch(&error_type, &error_value, &error_traceback);
+
+        if (PyErr_WarnFormat(PyExc_ResourceWarning, 1,
+                             "unclosed cursor %R for connection %R",
+                             obj, (PyObject *)self->conn))
+            PyErr_WriteUnraisable(obj);
+
+        /* Restore the saved exception. */
+        PyErr_Restore(error_type, error_value, error_traceback);
+    }
+}
+#endif
+
 static int
 cursor_init(PyObject *obj, PyObject *args, PyObject *kwargs)
 {
@@ -2056,7 +2082,7 @@ PyTypeObject cursorType = {
     0,          /*tp_setattro*/
     0,          /*tp_as_buffer*/
     Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_ITER |
-      Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_HAVE_WEAKREFS ,
+      Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_HAVE_WEAKREFS | Py_TPFLAGS_HAVE_FINALIZE,
                 /*tp_flags*/
     cursorType_doc, /*tp_doc*/
     (traverseproc)cursor_traverse, /*tp_traverse*/
@@ -2076,4 +2102,16 @@ PyTypeObject cursorType = {
     cursor_init, /*tp_init*/
     0,          /*tp_alloc*/
     cursor_new, /*tp_new*/
+    0,               /* tp_free */
+    0,               /* tp_is_gc */
+    0,               /* tp_bases */
+    0,               /* tp_mro */
+    0,               /* tp_cache */
+    0,               /* tp_subclasses */
+    0,               /* tp_weaklist */
+#if PY_3
+    0,               /* tp_del */
+    0,               /* tp_version_tag */
+    cursor_finalize, /* tp_finalize */
+#endif
 };

psycopg/python.h

@@ -86,6 +86,8 @@ typedef unsigned long Py_uhash_t;
 #define Bytes_ConcatAndDel PyString_ConcatAndDel
 #define _Bytes_Resize _PyString_Resize
 
+#define Py_TPFLAGS_HAVE_FINALIZE 0L
+
 #define PyDateTime_DELTA_GET_DAYS(o)         (((PyDateTime_Delta*)o)->days)
 #define PyDateTime_DELTA_GET_SECONDS(o)      (((PyDateTime_Delta*)o)->seconds)
 #define PyDateTime_DELTA_GET_MICROSECONDS(o) (((PyDateTime_Delta*)o)->microseconds)
@@ -97,6 +99,8 @@ typedef unsigned long Py_uhash_t;
     PyLong_FromUnsignedLong((unsigned long)(x)) : \
     PyInt_FromLong((x)))
 
+#define PyObject_CallFinalizerFromDealloc(obj) 0
+
 #endif  /* PY_2 */
 
 #if PY_3

tests/__init__.py

@@ -56,6 +56,7 @@ from . import test_sql
 from . import test_transaction
 from . import test_types_basic
 from . import test_types_extras
+from . import test_warnings
 from . import test_with
 
 if sys.version_info[:2] < (3, 6):
@@ -101,6 +102,7 @@ def test_suite():
     suite.addTest(test_transaction.test_suite())
     suite.addTest(test_types_basic.test_suite())
     suite.addTest(test_types_extras.test_suite())
+    suite.addTest(test_warnings.test_suite())
     suite.addTest(test_with.test_suite())
     return suite
 

tests/test_warnings.py

@@ -0,0 +1,63 @@
+import unittest
+import warnings
+
+import psycopg2
+
+from .testconfig import dsn
+from .testutils import skip_before_python
+
+
+class WarningsTest(unittest.TestCase):
+    @skip_before_python(3)
+    def test_connection_not_closed(self):
+        def f():
+            psycopg2.connect(dsn)
+
+        msg = (
+            "^unclosed connection <connection object at 0x[0-9a-f]+; dsn: '.*', "
+            "closed: 0>$"
+        )
+        with self.assertWarnsRegex(ResourceWarning, msg):
+            f()
+
+    @skip_before_python(3)
+    def test_cursor_not_closed(self):
+        def f():
+            conn = psycopg2.connect(dsn)
+            try:
+                conn.cursor()
+            finally:
+                conn.close()
+
+        msg = (
+            "^unclosed cursor <cursor object at 0x[0-9a-f]+; closed: 0> for "
+            "connection <connection object at 0x[0-9a-f]+; dsn: '.*', closed: 0>$"
+        )
+        with self.assertWarnsRegex(ResourceWarning, msg):
+            f()
+
+    def test_cursor_factory_returns_non_cursor(self):
+        def bad_factory(*args, **kwargs):
+            return object()
+
+        def f():
+            conn = psycopg2.connect(dsn)
+            try:
+                conn.cursor(cursor_factory=bad_factory)
+            finally:
+                conn.close()
+
+        with warnings.catch_warnings(record=True) as cm:
+            with self.assertRaises(TypeError):
+                f()
+
+        # No warning as no cursor was instantiated.
+        self.assertEquals(cm, [])
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)
+
+
+if __name__ == "__main__":
+    unittest.main()