From e2b7d0c65c611ae3044eb3a49ada71aa22cbca80 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 | 25 ++++++++++---- src/interfaces/libpq/fe-exec.c | 53 ++++++++++++++++------------- src/interfaces/libpq/libpq-events.c | 3 +- src/interfaces/libpq/libpq-int.h | 3 +- 4 files changed, 53 insertions(+), 31 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 2db369e906d..c5da033a330 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1,4 +1,4 @@ - + <application>libpq</application> - C Library @@ -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 7db303ce008..00889c8a7d7 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/interfaces/libpq/fe-exec.c,v 1.198 2008/09/17 04:31:08 tgl Exp $ + * $PostgreSQL: pgsql/src/interfaces/libpq/fe-exec.c,v 1.199 2008/09/19 16:40:40 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -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 7d3d1cb26c1..9f46336a58b 100644 --- a/src/interfaces/libpq/libpq-events.c +++ b/src/interfaces/libpq/libpq-events.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-events.c,v 1.1 2008/09/17 04:31:08 tgl Exp $ + * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-events.c,v 1.2 2008/09/19 16:40:40 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -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 fd29c092148..b29057bde95 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -12,7 +12,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-int.h,v 1.132 2008/09/17 04:31:08 tgl Exp $ + * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-int.h,v 1.133 2008/09/19 16:40:40 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -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