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

Commitb217459

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 parent1798dd1 commitb217459

File tree

2 files changed

+102
-66
lines changed

2 files changed

+102
-66
lines changed

‎contrib/postgres_fdw/connection.c

Lines changed: 92 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -494,48 +494,58 @@ pgfdw_exec_query(PGconn *conn, const char *query)
494494
*
495495
* This function offers quick responsiveness by checking for any interruptions.
496496
*
497-
* This function emulatesthePQexec()'s behavior of returning the last result
497+
* This function emulates PQexec()'s behavior of returning the last result
498498
* when there are many.
499499
*
500500
* Caller is responsible for the error handling on the result.
501501
*/
502502
PGresult*
503503
pgfdw_get_result(PGconn*conn,constchar*query)
504504
{
505-
PGresult*last_res=NULL;
505+
PGresult*volatilelast_res=NULL;
506506

507-
for (;;)
507+
/* In what follows, do not leak any PGresults on an error. */
508+
PG_TRY();
508509
{
509-
PGresult*res;
510-
511-
while (PQisBusy(conn))
510+
for (;;)
512511
{
513-
intwc;
512+
PGresult*res;
514513

515-
/* Sleep until there's something to do */
516-
wc=WaitLatchOrSocket(MyLatch,
517-
WL_LATCH_SET |WL_SOCKET_READABLE,
518-
PQsocket(conn),
519-
-1L);
520-
ResetLatch(MyLatch);
514+
while (PQisBusy(conn))
515+
{
516+
intwc;
521517

522-
CHECK_FOR_INTERRUPTS();
518+
/* Sleep until there's something to do */
519+
wc=WaitLatchOrSocket(MyLatch,
520+
WL_LATCH_SET |WL_SOCKET_READABLE,
521+
PQsocket(conn),
522+
-1L);
523+
ResetLatch(MyLatch);
523524

524-
/* Data available in socket */
525-
if (wc&WL_SOCKET_READABLE)
526-
{
527-
if (!PQconsumeInput(conn))
528-
pgfdw_report_error(ERROR,NULL,conn, false,query);
525+
CHECK_FOR_INTERRUPTS();
526+
527+
/* Data available in socket? */
528+
if (wc&WL_SOCKET_READABLE)
529+
{
530+
if (!PQconsumeInput(conn))
531+
pgfdw_report_error(ERROR,NULL,conn, false,query);
532+
}
529533
}
530-
}
531534

532-
res=PQgetResult(conn);
533-
if (res==NULL)
534-
break;/* query is complete */
535+
res=PQgetResult(conn);
536+
if (res==NULL)
537+
break;/* query is complete */
535538

539+
PQclear(last_res);
540+
last_res=res;
541+
}
542+
}
543+
PG_CATCH();
544+
{
536545
PQclear(last_res);
537-
last_res=res;
546+
PG_RE_THROW();
538547
}
548+
PG_END_TRY();
539549

540550
returnlast_res;
541551
}
@@ -1015,6 +1025,7 @@ pgfdw_exec_cleanup_query(PGconn *conn, const char *query, bool ignore_errors)
10151025
pgfdw_report_error(WARNING,result,conn, true,query);
10161026
returnignore_errors;
10171027
}
1028+
PQclear(result);
10181029

10191030
return true;
10201031
}
@@ -1037,56 +1048,75 @@ pgfdw_exec_cleanup_query(PGconn *conn, const char *query, bool ignore_errors)
10371048
staticbool
10381049
pgfdw_get_cleanup_result(PGconn*conn,TimestampTzendtime,PGresult**result)
10391050
{
1040-
PGresult*last_res=NULL;
1051+
volatilebooltimed_out= false;
1052+
PGresult*volatilelast_res=NULL;
10411053

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

1069-
CHECK_FOR_INTERRUPTS();
1087+
CHECK_FOR_INTERRUPTS();
10701088

1071-
/* Data available in socket */
1072-
if (wc&WL_SOCKET_READABLE)
1073-
{
1074-
if (!PQconsumeInput(conn))
1089+
/* Data available in socket? */
1090+
if (wc&WL_SOCKET_READABLE)
10751091
{
1076-
*result=NULL;
1077-
return false;
1092+
if (!PQconsumeInput(conn))
1093+
{
1094+
/* connection trouble; treat the same as a timeout */
1095+
timed_out= true;
1096+
gotoexit;
1097+
}
10781098
}
10791099
}
1080-
}
10811100

1082-
res=PQgetResult(conn);
1083-
if (res==NULL)
1084-
break;/* query is complete */
1101+
res=PQgetResult(conn);
1102+
if (res==NULL)
1103+
break;/* query is complete */
10851104

1105+
PQclear(last_res);
1106+
last_res=res;
1107+
}
1108+
exit:;
1109+
}
1110+
PG_CATCH();
1111+
{
10861112
PQclear(last_res);
1087-
last_res=res;
1113+
PG_RE_THROW();
10881114
}
1115+
PG_END_TRY();
10891116

1090-
*result=last_res;
1091-
return false;
1117+
if (timed_out)
1118+
PQclear(last_res);
1119+
else
1120+
*result=last_res;
1121+
returntimed_out;
10921122
}

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -478,14 +478,20 @@ libpqrcv_PQexec(const char *query)
478478
*/
479479
if (!libpq_select(-1))
480480
continue;/* interrupted */
481+
482+
/* Consume whatever data is available from the socket */
481483
if (PQconsumeInput(streamConn)==0)
482-
returnNULL;/* trouble */
484+
{
485+
/* trouble; drop whatever we had and return NULL */
486+
PQclear(lastResult);
487+
returnNULL;
488+
}
483489
}
484490

485491
/*
486-
* EmulatethePQexec()'s behavior of returning the last result when
487-
*thereare many. Since walsender will never generate multiple
488-
*results, weskip the concatenation of error messages.
492+
* Emulate PQexec()'s behavior of returning the last result when there
493+
* are many.Since walsender will never generate multiple results, we
494+
* skip the concatenation of error messages.
489495
*/
490496
result=PQgetResult(streamConn);
491497
if (result==NULL)

0 commit comments

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