This is an automated email from the ASF dual-hosted git repository. leborchuk pushed a commit to branch REL_2_STABLE in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit 392c4e2a03bee1e416c0572cb45d2ab7df8d2120 Author: Heikki Linnakangas <[email protected]> AuthorDate: Thu Nov 23 13:30:13 2023 +0200 Fix resource leak when a FDW's ForeignAsyncRequest function fails If an error is thrown after calling CreateWaitEventSet(), the memory of a WaitEventSet is free'd as it's allocated in the short-lived memory context, but the file descriptor (on epoll- or kqueue-based systems) or handles (on Windows) that it contains are leaked. Use PG_TRY-FINALLY to ensure it gets freed. (On master, I will apply a better fix, using ResourceOwners to track the WaitEventSet, but that's not backpatchable.) The added test doesn't check for leaking resources, so it passed even before this commit. But at least it covers the code path. In the passing, fix misleading comment on what the 'nevents' argument to WaitEventSetWait means. Report by Alexander Lakhin, analysis and suggestion for the fix by Tom Lane. Fixes bug #17828. Backpatch to v14 where async execution was introduced, but master gets a different fix. Discussion: https://www.postgresql.org/message-id/[email protected] Discussion: https://www.postgresql.org/message-id/[email protected] (cherry picked from commit 555276f8594087ba15e0d58e38cd2186b9f39f6d) --- contrib/postgres_fdw/expected/postgres_fdw.out | 7 +++ contrib/postgres_fdw/sql/postgres_fdw.sql | 6 +++ src/backend/executor/nodeAppend.c | 66 +++++++++++++++----------- 3 files changed, 50 insertions(+), 29 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index caa361a8df5..a9d516bc537 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -10359,6 +10359,13 @@ SELECT * FROM result_tbl ORDER BY a; (2 rows) DELETE FROM result_tbl; +-- Test error handling, if accessing one of the foreign partitions errors out +CREATE FOREIGN TABLE async_p_broken PARTITION OF async_pt FOR VALUES FROM (10000) TO (10001) + SERVER loopback OPTIONS (table_name 'non_existent_table'); +SELECT * FROM async_pt; +ERROR: relation "public.non_existent_table" does not exist +CONTEXT: remote SQL command: SELECT a, b, c FROM public.non_existent_table +DROP FOREIGN TABLE async_p_broken; -- Check case where multiple partitions use the same connection CREATE TABLE base_tbl3 (a int, b int, c text); CREATE FOREIGN TABLE async_p3 PARTITION OF async_pt FOR VALUES FROM (3000) TO (4000) diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 9a77a47f4e7..2c1e4c88e26 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -3246,6 +3246,12 @@ INSERT INTO result_tbl SELECT * FROM async_pt WHERE b === 505; SELECT * FROM result_tbl ORDER BY a; DELETE FROM result_tbl; +-- Test error handling, if accessing one of the foreign partitions errors out +CREATE FOREIGN TABLE async_p_broken PARTITION OF async_pt FOR VALUES FROM (10000) TO (10001) + SERVER loopback OPTIONS (table_name 'non_existent_table'); +SELECT * FROM async_pt; +DROP FOREIGN TABLE async_p_broken; + -- Check case where multiple partitions use the same connection CREATE TABLE base_tbl3 (a int, b int, c text); CREATE FOREIGN TABLE async_p3 PARTITION OF async_pt FOR VALUES FROM (3000) TO (4000) diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c index e54edf01ea5..a81fd36b9c4 100644 --- a/src/backend/executor/nodeAppend.c +++ b/src/backend/executor/nodeAppend.c @@ -1068,43 +1068,51 @@ ExecAppendAsyncEventWait(AppendState *node) /* We should never be called when there are no valid async subplans. */ Assert(node->as_nasyncremain > 0); + Assert(node->as_eventset == NULL); node->as_eventset = CreateWaitEventSet(CurrentMemoryContext, nevents); - AddWaitEventToSet(node->as_eventset, WL_EXIT_ON_PM_DEATH, PGINVALID_SOCKET, - NULL, NULL); - - /* Give each waiting subplan a chance to add an event. */ - i = -1; - while ((i = bms_next_member(node->as_asyncplans, i)) >= 0) + PG_TRY(); { - AsyncRequest *areq = node->as_asyncrequests[i]; + AddWaitEventToSet(node->as_eventset, WL_EXIT_ON_PM_DEATH, PGINVALID_SOCKET, + NULL, NULL); - if (areq->callback_pending) - ExecAsyncConfigureWait(areq); - } + /* Give each waiting subplan a chance to add an event. */ + i = -1; + while ((i = bms_next_member(node->as_asyncplans, i)) >= 0) + { + AsyncRequest *areq = node->as_asyncrequests[i]; - /* - * No need for further processing if there are no configured events other - * than the postmaster death event. - */ - if (GetNumRegisteredWaitEvents(node->as_eventset) == 1) + if (areq->callback_pending) + ExecAsyncConfigureWait(areq); + } + + /* + * No need for further processing if there are no configured events + * other than the postmaster death event. + */ + if (GetNumRegisteredWaitEvents(node->as_eventset) == 1) + { + FreeWaitEventSet(node->as_eventset); + node->as_eventset = NULL; + return; + } + + /* Return at most EVENT_BUFFER_SIZE events in one call. */ + if (nevents > EVENT_BUFFER_SIZE) + nevents = EVENT_BUFFER_SIZE; + + /* + * If the timeout is -1, wait until at least one event occurs. If the + * timeout is 0, poll for events, but do not wait at all. + */ + noccurred = WaitEventSetWait(node->as_eventset, timeout, occurred_event, + nevents, WAIT_EVENT_APPEND_READY); + } + PG_FINALLY(); { FreeWaitEventSet(node->as_eventset); node->as_eventset = NULL; - return; } - - /* We wait on at most EVENT_BUFFER_SIZE events. */ - if (nevents > EVENT_BUFFER_SIZE) - nevents = EVENT_BUFFER_SIZE; - - /* - * If the timeout is -1, wait until at least one event occurs. If the - * timeout is 0, poll for events, but do not wait at all. - */ - noccurred = WaitEventSetWait(node->as_eventset, timeout, occurred_event, - nevents, WAIT_EVENT_APPEND_READY); - FreeWaitEventSet(node->as_eventset); - node->as_eventset = NULL; + PG_END_TRY(); if (noccurred == 0) return; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
