AtAbort_Portsl problem - Mailing list pgsql-hackers

Hi,

While ago, I reported a problem regarding exec_execute_message crash
in transaction abort state if sync message issued right after parse,
bind and execute message (which is normal if used with pgpool). After
further investigation, I concluded that there's a serious problem with
unnamed portal handling.

The previous unnamed portal remains until next exec_bind_message
call(it calls CreatePortal which drops unnamed portal to replace it
with new one). If the next call to exec_parse_message fails by some
reasons (for example, parse error), the previous unnamed portal
remains with some data trashed by AtAbort_Portals. The reason why I
see exec_execute_message crash is, it looks into the broken portal,
especially portal->stmts:
is_xact_command = IsTransactionStmtList(portal->stmts);

unfortunately which was already freed by PortalReleaseCachedPlan call
in AtAbort_Portals because portal->cplan and portal->stmts belong to
the same memory context in some cases.

The reason why we don't see the problem until now is probably a) just
lucky b) libpq/drivers do not issue execute if parse fails. But bug is
bug anyway, I believe we need to fix this.

One of fixes I can think of is, set NULL to portal->stmts in
AtAbort_Portsl(see attached patches against CVS Head). This will not
result in memory leak since either portal->stmts belongs to the same
memory context of portal->cplan (in this case the memory context will
be dropped by PortalReleaseCachedPlan) or belongs to the same one of
portal heap meory, which will be dropped at the same time when the
portal itself dropped.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
Index: src/backend/utils/mmgr/portalmem.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/mmgr/portalmem.c,v
retrieving revision 1.115
diff -c -r1.115 portalmem.c
*** src/backend/utils/mmgr/portalmem.c    2 Jan 2010 16:57:58 -0000    1.115
--- src/backend/utils/mmgr/portalmem.c    17 Jan 2010 13:19:07 -0000
***************
*** 684,689 ****
--- 684,700 ----             PortalReleaseCachedPlan(portal);          /*
+          * Remove the reference to PlannedStmts and/or utility
+          * statements.  This is neccessary since
+          * PortalReleaseCachedPlan might release memory context which
+          * stmts belogs to and subsequent call to exec_execute_message
+          * looks into stmts to check if it's a transaction
+          * command(which is allow to call even if in an aborted
+          * transaction state), which might refer to freed memory.
+          */
+         portal->stmts = NULL;
+ 
+         /*          * Any resources belonging to the portal will be released in the          * upcoming
transaction-widecleanup; they will be gone before we run          * PortalDrop.