database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-08-30 06:01:21 +03:00
Code Activity

Fix resource leak when a FDW's ForeignAsyncRequest function fails

Browse Source 
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/17828-122da8cba23236be@postgresql.org
Discussion: https://www.postgresql.org/message-id/472235.1678387869@sss.pgh.pa.us
This commit is contained in:
Heikki Linnakangas
2023-11-23 13:30:13 +02:00
parent 57aae65aee
commit 481d7d1c01
3 changed files with 50 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

66
src/backend/executor/nodeAppend.c
View File

@@ -1016,43 +1016,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];
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);
}
/*
* No need for further processing if there are no configured events other
* than the postmaster death event.
*/
if (GetNumRegisteredWaitEvents(node->as_eventset) == 1)
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;