From 05659c0d16767f0cba55d0e6acfbe3748aacc719 Mon Sep 17 00:00:00 2001
From: Daniele Varrazzo <daniele.varrazzo@gmail.com>
Date: Sat, 4 Jun 2011 01:49:03 +0100
Subject: [PATCH] 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 | 19 +++++--------------
 psycopg/pqpath.c         | 30 +++++++++++++++++-------------
 2 files changed, 22 insertions(+), 27 deletions(-)

diff --git a/psycopg/connection_int.c b/psycopg/connection_int.c
index 18a0a14f..24d424df 100644
--- a/psycopg/connection_int.c
+++ b/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;
 
@@ -154,11 +148,8 @@ conn_notice_clean(connectionObject *self)
         free((void*)tmp->message);
         free(tmp);
     }
-    
+
     self->notice_pending = NULL;
-    
-    pthread_mutex_unlock(&self->lock);
-    Py_END_ALLOW_THREADS;    
 }
 
 
diff --git a/psycopg/pqpath.c b/psycopg/pqpath.c
index e1a4f5fb..f50b00b4 100644
--- a/psycopg/pqpath.c
+++ b/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) */