Cleanup of notice processing

The function is always called in the context of functions grabbing the
connection lock, so just use the same critical section instead of releasing
and re-acquiring it. It is not a problem as serious as the notifies process
(ticket #55) as the notices are a psycopg structure, not libpq. However the
change allows again processing notices/notifies in the same place,
which makes sense conceptually, plus we save some lock dance.

psycopg/connection_int.c

@@ -95,6 +95,10 @@ conn_notice_callback(void *args, const char *message)
     self->notice_pending = notice;
 }
 
+/* Expose the notices received as Python objects.
+ *
+ * The function should be called with the connection lock and the GIL.
+ */
 void
 conn_notice_process(connectionObject *self)
 {
@@ -105,10 +109,6 @@ conn_notice_process(connectionObject *self)
         return;
     }
 
-    Py_BEGIN_ALLOW_THREADS;
-    pthread_mutex_lock(&self->lock);
-    Py_BLOCK_THREADS;
-
     notice =  self->notice_pending;
     nnotices = PyList_GET_SIZE(self->notice_list);
 
@@ -132,10 +132,6 @@ conn_notice_process(connectionObject *self)
             0, nnotices - CONN_NOTICES_LIMIT);
     }
 
-    Py_UNBLOCK_THREADS;
-    pthread_mutex_unlock(&self->lock);
-    Py_END_ALLOW_THREADS;
-
     conn_notice_clean(self);
 }
 
@@ -143,8 +139,6 @@ void
 conn_notice_clean(connectionObject *self)
 {
     struct connectionObject_notice *tmp, *notice;
-    Py_BEGIN_ALLOW_THREADS;
-    pthread_mutex_lock(&self->lock);
 
     notice = self->notice_pending;
 
@@ -156,9 +150,6 @@ conn_notice_clean(connectionObject *self)
     }
 
     self->notice_pending = NULL;
-    
-    pthread_mutex_unlock(&self->lock);
-    Py_END_ALLOW_THREADS;    
 }
 
 

psycopg/pqpath.c

@@ -450,11 +450,13 @@ pq_commit(connectionObject *conn)
 
     retvalue = pq_execute_command_locked(conn, "COMMIT", &pgres, &error, &_save);
 
+    Py_BLOCK_THREADS;
+    conn_notice_process(conn);
+    Py_UNBLOCK_THREADS;
+
     pthread_mutex_unlock(&conn->lock);
     Py_END_ALLOW_THREADS;
 
-    conn_notice_process(conn);
-
     if (retvalue < 0)
         pq_complete_error(conn, &pgres, &error);
 
@@ -512,11 +514,13 @@ pq_abort(connectionObject *conn)
 
     retvalue = pq_abort_locked(conn, &pgres, &error, &_save);
 
+    Py_BLOCK_THREADS;
+    conn_notice_process(conn);
+    Py_UNBLOCK_THREADS;
+
     pthread_mutex_unlock(&conn->lock);
     Py_END_ALLOW_THREADS;
 
-    conn_notice_process(conn);
-
     if (retvalue < 0)
         pq_complete_error(conn, &pgres, &error);
 
@@ -576,11 +580,13 @@ pq_reset(connectionObject *conn)
 
     retvalue = pq_reset_locked(conn, &pgres, &error, &_save);
 
+    Py_BLOCK_THREADS;
+    conn_notice_process(conn);
+    Py_UNBLOCK_THREADS;
+
     pthread_mutex_unlock(&conn->lock);
     Py_END_ALLOW_THREADS;
 
-    conn_notice_process(conn);
-
     if (retvalue < 0) {
         pq_complete_error(conn, &pgres, &error);
     }
@@ -667,13 +673,12 @@ pq_is_busy(connectionObject *conn)
 
     Py_BLOCK_THREADS;
     conn_notifies_process(conn);
+    conn_notice_process(conn);
     Py_UNBLOCK_THREADS;
 
     pthread_mutex_unlock(&(conn->lock));
     Py_END_ALLOW_THREADS;
 
-    conn_notice_process(conn);
-
     return res;
 }
 
@@ -693,9 +698,9 @@ pq_is_busy_locked(connectionObject *conn)
         return -1;
     }
 
-    /* We can't call conn_notice_process/conn_notifies_process because
-      they try to get the lock. We don't need anyway them because at the end of
-      the loop we are in (async reading) pq_fetch will be called. */
+    /* notices and notifies will be processed at the end of the loop we are in
+     * (async reading) by pq_fetch. */
+
     return PQisBusy(conn->pgconn);
 }
 
@@ -791,6 +796,7 @@ pq_execute(cursorObject *curs, const char *query, int async)
          * (as in ticket #55). */
         Py_BLOCK_THREADS;
         conn_notifies_process(curs->conn);
+        conn_notice_process(curs->conn);
         Py_UNBLOCK_THREADS;
     }
 
@@ -1380,8 +1386,6 @@ pq_fetch(cursorObject *curs)
         break;
     }
 
-    conn_notice_process(curs->conn);
-
     /* error checking, close the connection if necessary (some critical errors
        are not really critical, like a COPY FROM error: if that's the case we
        raise the exception but we avoid to close the connection) */