From 56a6b6a72c76eef15f98962e3f85ebc30cc58bd7 Mon Sep 17 00:00:00 2001 From: Tatsuo Ishii Date: Thu, 28 Mar 2019 13:58:27 +0900 Subject: [PATCH] Fix memory leak in "batch" mode in extended query. In "batch" mode, not for every execute message, a sync message is followed. Unfortunately Pgpool-II only discard memory of query context for the last execute message while processing the ready for query message. For example if 3 execute messages are sent before the sync message, 2 of query context memory will not be freed and this leads to serious memory leak. To fix the problem, now the query context memory is possibly discarded when a command complete message is returned from backend if the query context is not referenced either by sent messages or pending messages. If it is not referenced at all, we can discard the query context. Also even if it is referenced, it is ok to discard the query context if it is either an unnamed statement or an unnamed portal because it will be discarded anyway when next unnamed statement or portal is created. Per bug 468. --- src/context/pool_query_context.c | 3 +- src/context/pool_session_context.c | 49 +++++++++++++++++++++- src/include/context/pool_session_context.h | 2 + src/protocol/CommandComplete.c | 25 +++++++++++ 4 files changed, 77 insertions(+), 2 deletions(-) diff --git a/src/context/pool_query_context.c b/src/context/pool_query_context.c index 1d5ed1975..b82da454d 100644 --- a/src/context/pool_query_context.c +++ b/src/context/pool_query_context.c @@ -90,7 +90,8 @@ pool_query_context_destroy(POOL_QUERY_CONTEXT * query_context) MemoryContext memory_context = query_context->memory_context; ereport(DEBUG5, - (errmsg("pool_query_context_destroy: query context:%p", query_context))); + (errmsg("pool_query_context_destroy: query context:%p query: \"%s\"", + query_context, query_context->original_query))); session_context = pool_get_session_context(false); pool_unset_query_in_progress(); diff --git a/src/context/pool_session_context.c b/src/context/pool_session_context.c index 2d19fa9ea..7634acf32 100644 --- a/src/context/pool_session_context.c +++ b/src/context/pool_session_context.c @@ -484,6 +484,30 @@ pool_clear_sent_message_list(void) } } +/* + * Zap query context info in sent messages to indicate that the query context + * has been already removed. + */ +void +pool_zap_query_context_in_sent_messages(POOL_QUERY_CONTEXT *query_context) +{ + int i; + POOL_SENT_MESSAGE_LIST *msglist; + + msglist = &pool_get_session_context(false)->message_list; + + for (i = 0; i < msglist->size; i++) + { + elog(LOG, "checking zapping sent message: %p query_context: %p", + &msglist->sent_messages[i], msglist->sent_messages[i]->query_context); + if (msglist->sent_messages[i]->query_context == query_context) + { + msglist->sent_messages[i]->query_context = NULL; + elog(LOG, "Zap sent message: %p", &msglist->sent_messages[i]); + } + } +} + static void dump_sent_message(char *caller, POOL_SENT_MESSAGE * m) { @@ -597,7 +621,7 @@ pool_add_sent_message(POOL_SENT_MESSAGE * message) } /* - * Get a sent message + * Find a sent message by kind and name. */ POOL_SENT_MESSAGE * pool_get_sent_message(char kind, const char *name, POOL_SENT_MESSAGE_STATE state) @@ -621,6 +645,29 @@ pool_get_sent_message(char kind, const char *name, POOL_SENT_MESSAGE_STATE state return NULL; } +/* + * Find a sent message by query context. + */ +POOL_SENT_MESSAGE * +pool_get_sent_message_by_query_context(POOL_QUERY_CONTEXT * query_context) +{ + int i; + POOL_SENT_MESSAGE_LIST *msglist; + + msglist = &pool_get_session_context(false)->message_list; + + if (query_context == NULL) + return NULL; + + for (i = 0; i < msglist->size; i++) + { + if (msglist->sent_messages[i]->query_context == query_context) + return msglist->sent_messages[i]; + } + + return NULL; +} + /* * Set message state to POOL_SENT_MESSAGE_STATE to POOL_SENT_MESSAGE_CLOSED. */ diff --git a/src/include/context/pool_session_context.h b/src/include/context/pool_session_context.h index b701551c9..5f495d19b 100644 --- a/src/include/context/pool_session_context.h +++ b/src/include/context/pool_session_context.h @@ -297,6 +297,8 @@ extern void pool_clear_sent_message_list(void); extern void pool_sent_message_destroy(POOL_SENT_MESSAGE * message); extern POOL_SENT_MESSAGE * pool_get_sent_message(char kind, const char *name, POOL_SENT_MESSAGE_STATE state); extern void pool_set_sent_message_state(POOL_SENT_MESSAGE * message); +extern void pool_zap_query_context_in_sent_messages(POOL_QUERY_CONTEXT *query_context); +extern POOL_SENT_MESSAGE * pool_get_sent_message_by_query_context(POOL_QUERY_CONTEXT * query_context); extern void pool_unset_writing_transaction(void); extern void pool_set_writing_transaction(void); extern bool pool_is_writing_transaction(void); diff --git a/src/protocol/CommandComplete.c b/src/protocol/CommandComplete.c index 709f96c7e..d5e97a413 100644 --- a/src/protocol/CommandComplete.c +++ b/src/protocol/CommandComplete.c @@ -216,6 +216,31 @@ CommandComplete(POOL_CONNECTION * frontend, POOL_CONNECTION_POOL * backend, bool pool_at_command_success(frontend, backend); pool_unset_query_in_progress(); pool_pending_message_reset_previous_message(); + + if (session_context->query_context == NULL) + { + elog(WARNING, "At command complete there's no query context"); + } + else + { + /* + * Destroy query context if it is not referenced from sent + * messages and pending messages except bound to named statements + * or portals. Named statements and portals should remain until + * they are explicitly closed. + */ + if (can_query_context_destroy(session_context->query_context)) + + { + POOL_SENT_MESSAGE * msg = pool_get_sent_message_by_query_context(session_context->query_context); + + if (!msg || (msg && *msg->name == '\0')) + { + pool_zap_query_context_in_sent_messages(session_context->query_context); + pool_query_context_destroy(session_context->query_context); + } + } + } } return POOL_CONTINUE; -- 2.39.5