diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c index 64d0492b83e3c3d3ddb8fe43c8ae504edf1a35b3..83998233b49dcba79a50a7f5221d48c8b0e01c24 100644 --- a/src/backend/commands/portalcmds.c +++ b/src/backend/commands/portalcmds.c @@ -14,7 +14,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.72 2008/03/26 18:48:59 alvherre Exp $ + * $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.73 2008/04/02 18:31:50 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -77,6 +77,9 @@ PerformCursorOpen(PlannedStmt *stmt, ParamListInfo params, stmt = copyObject(stmt); stmt->utilityStmt = NULL; /* make it look like plain SELECT */ + if (queryString) /* copy the source text too for safety */ + queryString = pstrdup(queryString); + PortalDefineQuery(portal, NULL, queryString, diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c index 681647fac56111aaed90f6f2b126425197ebec9a..575dad9fb3fad980b4b635c1f1a46cce4ead2104 100644 --- a/src/backend/commands/prepare.c +++ b/src/backend/commands/prepare.c @@ -10,7 +10,7 @@ * Copyright (c) 2002-2008, PostgreSQL Global Development Group * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.84 2008/03/26 18:48:59 alvherre Exp $ + * $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.85 2008/04/02 18:31:50 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -250,9 +250,13 @@ ExecuteQuery(ExecuteStmt *stmt, const char *queryString, plan_list = cplan->stmt_list; } + /* + * Note: we don't bother to copy the source query string into the portal. + * Any errors it might be useful for will already have been reported. + */ PortalDefineQuery(portal, NULL, - entry->plansource->query_string, + NULL, entry->plansource->commandTag, plan_list, cplan); diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 42187fe43ad07812248487049365f8e27cc4e455..f7d41373b4ad8e286af5ab85132560e3176f7012 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.192 2008/04/01 03:09:30 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.193 2008/04/02 18:31:50 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -918,6 +918,7 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan, CachedPlanSource *plansource; CachedPlan *cplan; List *stmt_list; + char *query_string; ParamListInfo paramLI; Snapshot snapshot; MemoryContext oldcontext; @@ -968,10 +969,22 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan, portal = CreatePortal(name, false, false); } + /* + * Prepare to copy stuff into the portal's memory context. We do all this + * copying first, because it could possibly fail (out-of-memory) and we + * don't want a failure to occur between RevalidateCachedPlan and + * PortalDefineQuery; that would result in leaking our plancache refcount. + */ + oldcontext = MemoryContextSwitchTo(PortalGetHeapMemory(portal)); + + /* Copy the plan's query string, if available, into the portal */ + query_string = plansource->query_string; + if (query_string) + query_string = pstrdup(query_string); + /* If the plan has parameters, copy them into the portal */ if (plan->nargs > 0) { - oldcontext = MemoryContextSwitchTo(PortalGetHeapMemory(portal)); /* sizeof(ParamListInfoData) includes the first array element */ paramLI = (ParamListInfo) palloc(sizeof(ParamListInfoData) + (plan->nargs - 1) *sizeof(ParamExternData)); @@ -1000,11 +1013,12 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan, paramTypByVal, paramTypLen); } } - MemoryContextSwitchTo(oldcontext); } else paramLI = NULL; + MemoryContextSwitchTo(oldcontext); + if (plan->saved) { /* Replan if needed, and increment plan refcount for portal */ @@ -1025,7 +1039,7 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan, */ PortalDefineQuery(portal, NULL, /* no statement name */ - plansource->query_string, + query_string, plansource->commandTag, stmt_list, cplan); diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 95bea2ef8a9e6fc10f70cbcbfaeaf0a99c8cea27..d44e69fb10af1a62cf627a823ef94f9366d1a088 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.547 2008/03/26 18:48:59 alvherre Exp $ + * $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.548 2008/04/02 18:31:50 tgl Exp $ * * NOTES * this is the "main" module of the postgres backend and @@ -933,6 +933,11 @@ exec_simple_query(const char *query_string) /* Don't display the portal in pg_cursors */ portal->visible = false; + /* + * We don't have to copy anything into the portal, because everything + * we are passsing here is in MessageContext, which will outlive the + * portal anyway. + */ PortalDefineQuery(portal, NULL, query_string, @@ -1356,8 +1361,11 @@ exec_bind_message(StringInfo input_message) CachedPlanSource *psrc; CachedPlan *cplan; Portal portal; + char *query_string; + char *saved_stmt_name; ParamListInfo params; List *plan_list; + MemoryContext oldContext; bool save_log_statement_stats = log_statement_stats; char msec_str[32]; @@ -1461,16 +1469,32 @@ exec_bind_message(StringInfo input_message) else portal = CreatePortal(portal_name, false, false); + /* + * Prepare to copy stuff into the portal's memory context. We do all this + * copying first, because it could possibly fail (out-of-memory) and we + * don't want a failure to occur between RevalidateCachedPlan and + * PortalDefineQuery; that would result in leaking our plancache refcount. + */ + oldContext = MemoryContextSwitchTo(PortalGetHeapMemory(portal)); + + /* Copy the plan's query string, if available, into the portal */ + query_string = psrc->query_string; + if (query_string) + query_string = pstrdup(query_string); + + /* Likewise make a copy of the statement name, unless it's unnamed */ + if (stmt_name[0]) + saved_stmt_name = pstrdup(stmt_name); + else + saved_stmt_name = NULL; + /* * Fetch parameters, if any, and store in the portal's memory context. */ if (numParams > 0) { - MemoryContext oldContext; int paramno; - oldContext = MemoryContextSwitchTo(PortalGetHeapMemory(portal)); - /* sizeof(ParamListInfoData) includes the first array element */ params = (ParamListInfo) palloc(sizeof(ParamListInfoData) + (numParams - 1) *sizeof(ParamExternData)); @@ -1595,12 +1619,13 @@ exec_bind_message(StringInfo input_message) params->params[paramno].pflags = PARAM_FLAG_CONST; params->params[paramno].ptype = ptype; } - - MemoryContextSwitchTo(oldContext); } else params = NULL; + /* Done storing stuff in portal's context */ + MemoryContextSwitchTo(oldContext); + /* Get the result format codes */ numRFormats = pq_getmsgint(input_message, 2); if (numRFormats > 0) @@ -1627,7 +1652,6 @@ exec_bind_message(StringInfo input_message) } else { - MemoryContext oldContext; List *query_list; /* @@ -1665,8 +1689,8 @@ exec_bind_message(StringInfo input_message) * Define portal and start execution. */ PortalDefineQuery(portal, - stmt_name[0] ? stmt_name : NULL, - psrc->query_string, + saved_stmt_name, + query_string, psrc->commandTag, plan_list, cplan); diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c index 4940f7721f68447b203fedbc62350708b3d1b794..88bad32e29e013fc23f12cc1cb4bfbca6d3a07b8 100644 --- a/src/backend/utils/mmgr/portalmem.c +++ b/src/backend/utils/mmgr/portalmem.c @@ -12,7 +12,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.108 2008/03/25 22:42:45 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.109 2008/04/02 18:31:50 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -274,9 +274,7 @@ CreateNewPortal(void) * * Notes: commandTag shall be NULL if and only if the original query string * (before rewriting) was an empty string. Also, the passed commandTag must - * be a pointer to a constant string, since it is not copied. However, - * prepStmtName and sourceText, if provided, are copied into the portal's - * heap context for safekeeping. + * be a pointer to a constant string, since it is not copied. * * If cplan is provided, then it is a cached plan containing the stmts, * and the caller must have done RevalidateCachedPlan(), causing a refcount @@ -285,6 +283,14 @@ CreateNewPortal(void) * If cplan is NULL, then it is the caller's responsibility to ensure that * the passed plan trees have adequate lifetime. Typically this is done by * copying them into the portal's heap context. + * + * The caller is also responsible for ensuring that the passed prepStmtName + * and sourceText (if not NULL) have adequate lifetime. + * + * NB: this function mustn't do much beyond storing the passed values; in + * particular don't do anything that risks elog(ERROR). If that were to + * happen here before storing the cplan reference, we'd leak the plancache + * refcount that the caller is trying to hand off to us. */ void PortalDefineQuery(Portal portal, @@ -299,10 +305,8 @@ PortalDefineQuery(Portal portal, Assert(commandTag != NULL || stmts == NIL); - portal->prepStmtName = prepStmtName ? - MemoryContextStrdup(PortalGetHeapMemory(portal), prepStmtName) : NULL; - portal->sourceText = sourceText ? - MemoryContextStrdup(PortalGetHeapMemory(portal), sourceText) : NULL; + portal->prepStmtName = prepStmtName; + portal->sourceText = sourceText; portal->commandTag = commandTag; portal->stmts = stmts; portal->cplan = cplan;