Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commita3bed62

Browse files
committed
Fix low-probability leaks of PGresult objects in the backend.
We had three occurrences of essentially the same coding patternwherein we tried to retrieve a query result from a libpq connectionwithout blocking. In the case where PQconsumeInput failed (typicallyindicating a lost connection), all three loops simply gave up andreturned, forgetting to clear any previously-collected PGresultobject. Since those are malloc'd not palloc'd, the oversight resultsin a process-lifespan memory leak.One instance, in libpqwalreceiver, is of little significance becausethe walreceiver process would just quit anyway if its connection fails.But we might as well fix it.The other two instances, in postgres_fdw, are somewhat more worrisomebecause at least in principle the scenario could be repeated, allowingthe amount of memory leaked to build up to something worth worryingabout. Moreover, in these cases the loops contain CHECK_FOR_INTERRUPTScalls, as well as other calls that could potentially elog(ERROR),providing another way to exit without having cleared the PGresult.Here we need to add PG_TRY logic similar to what exists in quite afew other places in postgres_fdw.Coverity noted the libpqwalreceiver bug; I found the other two casesby checking all calls of PQconsumeInput.Back-patch to all supported versions as appropriate (9.2 lackspostgres_fdw, so this is really quite unexciting for that branch).Discussion:https://postgr.es/m/22620.1497486981@sss.pgh.pa.us
1 parent07fb943 commita3bed62

File tree

2 files changed

+101
-65
lines changed

2 files changed

+101
-65
lines changed

‎contrib/postgres_fdw/connection.c

Lines changed: 92 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -485,48 +485,58 @@ pgfdw_exec_query(PGconn *conn, const char *query)
485485
*
486486
* This function offers quick responsiveness by checking for any interruptions.
487487
*
488-
* This function emulatesthePQexec()'s behavior of returning the last result
488+
* This function emulates PQexec()'s behavior of returning the last result
489489
* when there are many.
490490
*
491491
* Caller is responsible for the error handling on the result.
492492
*/
493493
PGresult*
494494
pgfdw_get_result(PGconn*conn,constchar*query)
495495
{
496-
PGresult*last_res=NULL;
496+
PGresult*volatilelast_res=NULL;
497497

498-
for (;;)
498+
/* In what follows, do not leak any PGresults on an error. */
499+
PG_TRY();
499500
{
500-
PGresult*res;
501-
502-
while (PQisBusy(conn))
501+
for (;;)
503502
{
504-
intwc;
503+
PGresult*res;
505504

506-
/* Sleep until there's something to do */
507-
wc=WaitLatchOrSocket(MyLatch,
508-
WL_LATCH_SET |WL_SOCKET_READABLE,
509-
PQsocket(conn),
510-
-1L,PG_WAIT_EXTENSION);
511-
ResetLatch(MyLatch);
505+
while (PQisBusy(conn))
506+
{
507+
intwc;
512508

513-
CHECK_FOR_INTERRUPTS();
509+
/* Sleep until there's something to do */
510+
wc=WaitLatchOrSocket(MyLatch,
511+
WL_LATCH_SET |WL_SOCKET_READABLE,
512+
PQsocket(conn),
513+
-1L,PG_WAIT_EXTENSION);
514+
ResetLatch(MyLatch);
514515

515-
/* Data available in socket */
516-
if (wc&WL_SOCKET_READABLE)
517-
{
518-
if (!PQconsumeInput(conn))
519-
pgfdw_report_error(ERROR,NULL,conn, false,query);
516+
CHECK_FOR_INTERRUPTS();
517+
518+
/* Data available in socket? */
519+
if (wc&WL_SOCKET_READABLE)
520+
{
521+
if (!PQconsumeInput(conn))
522+
pgfdw_report_error(ERROR,NULL,conn, false,query);
523+
}
520524
}
521-
}
522525

523-
res=PQgetResult(conn);
524-
if (res==NULL)
525-
break;/* query is complete */
526+
res=PQgetResult(conn);
527+
if (res==NULL)
528+
break;/* query is complete */
526529

530+
PQclear(last_res);
531+
last_res=res;
532+
}
533+
}
534+
PG_CATCH();
535+
{
527536
PQclear(last_res);
528-
last_res=res;
537+
PG_RE_THROW();
529538
}
539+
PG_END_TRY();
530540

531541
returnlast_res;
532542
}
@@ -1006,6 +1016,7 @@ pgfdw_exec_cleanup_query(PGconn *conn, const char *query, bool ignore_errors)
10061016
pgfdw_report_error(WARNING,result,conn, true,query);
10071017
returnignore_errors;
10081018
}
1019+
PQclear(result);
10091020

10101021
return true;
10111022
}
@@ -1028,56 +1039,75 @@ pgfdw_exec_cleanup_query(PGconn *conn, const char *query, bool ignore_errors)
10281039
staticbool
10291040
pgfdw_get_cleanup_result(PGconn*conn,TimestampTzendtime,PGresult**result)
10301041
{
1031-
PGresult*last_res=NULL;
1042+
volatilebooltimed_out= false;
1043+
PGresult*volatilelast_res=NULL;
10321044

1033-
for (;;)
1045+
/* In what follows, do not leak any PGresults on an error. */
1046+
PG_TRY();
10341047
{
1035-
PGresult*res;
1036-
1037-
while (PQisBusy(conn))
1048+
for (;;)
10381049
{
1039-
intwc;
1040-
TimestampTznow=GetCurrentTimestamp();
1041-
longsecs;
1042-
intmicrosecs;
1043-
longcur_timeout;
1044-
1045-
/* If timeout has expired, give up, else get sleep time. */
1046-
if (now >=endtime)
1047-
return true;
1048-
TimestampDifference(now,endtime,&secs,&microsecs);
1049-
1050-
/* To protect against clock skew, limit sleep to one minute. */
1051-
cur_timeout=Min(60000,secs*USECS_PER_SEC+microsecs);
1052-
1053-
/* Sleep until there's something to do */
1054-
wc=WaitLatchOrSocket(MyLatch,
1050+
PGresult*res;
1051+
1052+
while (PQisBusy(conn))
1053+
{
1054+
intwc;
1055+
TimestampTznow=GetCurrentTimestamp();
1056+
longsecs;
1057+
intmicrosecs;
1058+
longcur_timeout;
1059+
1060+
/* If timeout has expired, give up, else get sleep time. */
1061+
if (now >=endtime)
1062+
{
1063+
timed_out= true;
1064+
gotoexit;
1065+
}
1066+
TimestampDifference(now,endtime,&secs,&microsecs);
1067+
1068+
/* To protect against clock skew, limit sleep to one minute. */
1069+
cur_timeout=Min(60000,secs*USECS_PER_SEC+microsecs);
1070+
1071+
/* Sleep until there's something to do */
1072+
wc=WaitLatchOrSocket(MyLatch,
10551073
WL_LATCH_SET |WL_SOCKET_READABLE |WL_TIMEOUT,
1056-
PQsocket(conn),
1057-
cur_timeout,PG_WAIT_EXTENSION);
1058-
ResetLatch(MyLatch);
1074+
PQsocket(conn),
1075+
cur_timeout,PG_WAIT_EXTENSION);
1076+
ResetLatch(MyLatch);
10591077

1060-
CHECK_FOR_INTERRUPTS();
1078+
CHECK_FOR_INTERRUPTS();
10611079

1062-
/* Data available in socket */
1063-
if (wc&WL_SOCKET_READABLE)
1064-
{
1065-
if (!PQconsumeInput(conn))
1080+
/* Data available in socket? */
1081+
if (wc&WL_SOCKET_READABLE)
10661082
{
1067-
*result=NULL;
1068-
return false;
1083+
if (!PQconsumeInput(conn))
1084+
{
1085+
/* connection trouble; treat the same as a timeout */
1086+
timed_out= true;
1087+
gotoexit;
1088+
}
10691089
}
10701090
}
1071-
}
10721091

1073-
res=PQgetResult(conn);
1074-
if (res==NULL)
1075-
break;/* query is complete */
1092+
res=PQgetResult(conn);
1093+
if (res==NULL)
1094+
break;/* query is complete */
10761095

1096+
PQclear(last_res);
1097+
last_res=res;
1098+
}
1099+
exit:;
1100+
}
1101+
PG_CATCH();
1102+
{
10771103
PQclear(last_res);
1078-
last_res=res;
1104+
PG_RE_THROW();
10791105
}
1106+
PG_END_TRY();
10801107

1081-
*result=last_res;
1082-
return false;
1108+
if (timed_out)
1109+
PQclear(last_res);
1110+
else
1111+
*result=last_res;
1112+
returntimed_out;
10831113
}

‎src/backend/replication/libpqwalreceiver/libpqwalreceiver.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -591,13 +591,19 @@ libpqrcv_PQexec(PGconn *streamConn, const char *query)
591591
ResetLatch(MyLatch);
592592
CHECK_FOR_INTERRUPTS();
593593
}
594+
595+
/* Consume whatever data is available from the socket */
594596
if (PQconsumeInput(streamConn)==0)
595-
returnNULL;/* trouble */
597+
{
598+
/* trouble; drop whatever we had and return NULL */
599+
PQclear(lastResult);
600+
returnNULL;
601+
}
596602
}
597603

598604
/*
599-
* EmulatethePQexec()'s behavior of returning the last result when
600-
*thereare many. We are fine with returning just last error message.
605+
* Emulate PQexec()'s behavior of returning the last result when there
606+
* are many. We are fine with returning just last error message.
601607
*/
602608
result=PQgetResult(streamConn);
603609
if (result==NULL)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp