From 4160853ee21021b864b1b612444ad4a52747ec33 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 19 Sep 2008 16:40:40 +0000 Subject: [PATCH] Improve the recently-added libpq events code to provide more consistent guarantees about whether event procedures will receive DESTROY events. They no longer need to defend themselves against getting a DESTROY without a successful prior CREATE. Andrew Chernow --- doc/src/sgml/libpq.sgml | 23 ++++++++++--- src/interfaces/libpq/fe-exec.c | 51 ++++++++++++++++------------- src/interfaces/libpq/libpq-events.c | 1 + src/interfaces/libpq/libpq-int.h | 1 + 4 files changed, 49 insertions(+), 27 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 431608ba35..b72124b765 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -4914,7 +4914,9 @@ typedef struct PGconn that should be in the CONNECTION_OK status; guaranteed if one calls PQregisterEventProc right after obtaining a good - PGconn. + PGconn. When returning a failure code, all + cleanup must be performed as no PGEVT_CONNDESTROY + event will be sent. @@ -4944,7 +4946,10 @@ typedef struct PGEventConnReset *. Although the contained PGconn was just reset, all event data remains unchanged. This event should be used to reset/reload/requery any - associated instanceData. + associated instanceData. Note that even if the + event procedure fails to process PGEVT_CONNRESET, it will + still receive a PGEVT_CONNDESTROY event when the connection + is closed. @@ -5003,7 +5008,9 @@ typedef struct instanceData that needs to be associated with the result. If the event procedure fails, the result will be cleared and the failure will be propagated. The event procedure must not try to - PQclear the result object for itself. + PQclear the result object for itself. When returning a + failure code, all cleanup must be performed as no + PGEVT_RESULTDESTROY event will be sent. @@ -5014,7 +5021,10 @@ typedef struct The result copy event is fired in response to PQcopyResult. This event will only be fired after - the copy is complete. + the copy is complete. Only event procedures that have + successfully handled the PGEVT_RESULTCREATE + or PGEVT_RESULTCOPY event for the source result + will receive PGEVT_RESULTCOPY events. typedef struct @@ -5032,7 +5042,10 @@ typedef struct can be used to provide a deep copy of instanceData, since PQcopyResult cannot do that. If the event procedure fails, the entire copy operation will fail and the - dest result will be cleared. + dest result will be cleared. When returning a + failure code, all cleanup must be performed as no + PGEVT_RESULTDESTROY event will be sent for the + destination result. diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index 2ea5a679f7..062b5eeb63 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -331,10 +331,7 @@ PQcopyResult(const PGresult *src, int flags) if (flags & PG_COPYRES_NOTICEHOOKS) dest->noticeHooks = src->noticeHooks; - /* - * Wants to copy PGEvents? NB: this should be last, as we don't want - * to trigger RESULTDESTROY events on a useless PGresult. - */ + /* Wants to copy PGEvents? */ if ((flags & PG_COPYRES_EVENTS) && src->nEvents > 0) { dest->events = dupEvents(src->events, src->nEvents); @@ -349,15 +346,19 @@ PQcopyResult(const PGresult *src, int flags) /* Okay, trigger PGEVT_RESULTCOPY event */ for (i = 0; i < dest->nEvents; i++) { - PGEventResultCopy evt; - - evt.src = src; - evt.dest = dest; - if (!dest->events[i].proc(PGEVT_RESULTCOPY, &evt, - dest->events[i].passThrough)) + if (src->events[i].resultInitialized) { - PQclear(dest); - return NULL; + PGEventResultCopy evt; + + evt.src = src; + evt.dest = dest; + if (!dest->events[i].proc(PGEVT_RESULTCOPY, &evt, + dest->events[i].passThrough)) + { + PQclear(dest); + return NULL; + } + dest->events[i].resultInitialized = TRUE; } } @@ -365,8 +366,9 @@ PQcopyResult(const PGresult *src, int flags) } /* - * Copy an array of PGEvents (with no extra space for more) - * Does not duplicate the event instance data, sets this to NULL + * Copy an array of PGEvents (with no extra space for more). + * Does not duplicate the event instance data, sets this to NULL. + * Also, the resultInitialized flags are all cleared. */ static PGEvent * dupEvents(PGEvent *events, int count) @@ -381,13 +383,13 @@ dupEvents(PGEvent *events, int count) if (!newEvents) return NULL; - memcpy(newEvents, events, count * sizeof(PGEvent)); - - /* NULL out the data pointers and deep copy names */ for (i = 0; i < count; i++) { + newEvents[i].proc = events[i].proc; + newEvents[i].passThrough = events[i].passThrough; newEvents[i].data = NULL; - newEvents[i].name = strdup(newEvents[i].name); + newEvents[i].resultInitialized = FALSE; + newEvents[i].name = strdup(events[i].name); if (!newEvents[i].name) { while (--i >= 0) @@ -666,11 +668,15 @@ PQclear(PGresult *res) for (i = 0; i < res->nEvents; i++) { - PGEventResultDestroy evt; + /* only send DESTROY to successfully-initialized event procs */ + if (res->events[i].resultInitialized) + { + PGEventResultDestroy evt; - evt.result = res; - (void) res->events[i].proc(PGEVT_RESULTDESTROY, &evt, - res->events[i].passThrough); + evt.result = res; + (void) res->events[i].proc(PGEVT_RESULTDESTROY, &evt, + res->events[i].passThrough); + } free(res->events[i].name); } @@ -1612,6 +1618,7 @@ PQgetResult(PGconn *conn) res->resultStatus = PGRES_FATAL_ERROR; break; } + res->events[i].resultInitialized = TRUE; } } diff --git a/src/interfaces/libpq/libpq-events.c b/src/interfaces/libpq/libpq-events.c index d776783a05..8a894ff89e 100644 --- a/src/interfaces/libpq/libpq-events.c +++ b/src/interfaces/libpq/libpq-events.c @@ -76,6 +76,7 @@ PQregisterEventProc(PGconn *conn, PGEventProc proc, return FALSE; conn->events[conn->nEvents].passThrough = passThrough; conn->events[conn->nEvents].data = NULL; + conn->events[conn->nEvents].resultInitialized = FALSE; conn->nEvents++; regevt.conn = conn; diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 984bf59480..577bc7fac2 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -156,6 +156,7 @@ typedef struct PGEvent char *name; /* used only for error messages */ void *passThrough; /* pointer supplied at registration time */ void *data; /* optional state (instance) data */ + bool resultInitialized; /* T if RESULTCREATE/COPY succeeded */ } PGEvent; struct pg_result -- 2.39.5