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

Commitf6f0542

Browse files
hlinnakaJelteF
andcommitted
libpq: Handle OOM by disconnecting instead of hanging or skipping msgs
In most cases, if an out-of-memory situation happens, we attach theerror message to the connection and report it at the nextPQgetResult() call. However, there are a few cases, while processingmessages that are not associated with any particular query, where wehandled failed allocations differently and not very nicely:- If we ran out of memory while processing an async notification, getNotify() either returned EOF, which stopped processing any further data until more data was received from the server, or silently dropped the notification. Returning EOF is problematic because if more data never arrives, e.g. because the connection was used just to wait for the notification, or because the next ReadyForQuery was already received and buffered, it would get stuck forever. Silently dropping a notification is not nice either.- (New in v18) If we ran out of memory while receiving BackendKeyData message, getBackendKeyData() returned EOF, which has the same issues as in getNotify().- If we ran out of memory while saving a received a ParameterStatus message, we just skipped it. A later call to PQparameterStatus() would return NULL, even though the server did send the status.Change all those cases to terminate the connnection instead. Ouroptions for reporting those errors are limited, but it seems better toterminate than try to soldier on. Applications should handleconnection loss gracefully, whereas silently missing a notification,parameter status, or cancellation key could cause much weirderproblems.This also changes the error message on OOM while expanding the inputbuffer. It used to report "cannot allocate memory for input buffer",followed by "lost synchronization with server: got message type ...".The "lost synchronization" message seems unnecessary, so remove thatand report only "cannot allocate memory for input buffer". (Thecomment speculated that the out of memory could indeed be caused byloss of sync, but that seems highly unlikely.)This evolved from a more narrow patch by Jelte Fennema-Nio, which wasreviewed by Jacob Champion.Somewhat arbitrarily, backpatch to v18 but no further. These arelong-standing issues, but we haven't received any complaints from thefield. We can backpatch more later, if needed.Co-authored-by: Jelte Fennema-Nio <postgres@jeltef.nl>Reviewed-by: Jelte Fennema-Nio <postgres@jeltef.nl>Reviewed-by: Jacob Champion <jchampion@postgresql.org>Discussion:https://www.postgresql.org/message-id/df892f9f-5923-4046-9d6f-8c48d8980b50@iki.fiBackpatch-through: 18
1 parent661f821 commitf6f0542

File tree

3 files changed

+89
-40
lines changed

3 files changed

+89
-40
lines changed

‎src/interfaces/libpq/fe-exec.c‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1076,8 +1076,12 @@ pqSaveMessageField(PGresult *res, char code, const char *value)
10761076

10771077
/*
10781078
* pqSaveParameterStatus - remember parameter status sent by backend
1079+
*
1080+
* Returns 1 on success, 0 on out-of-memory. (Note that on out-of-memory, we
1081+
* have already released the old value of the parameter, if any. The only
1082+
* really safe way to recover is to terminate the connection.)
10791083
*/
1080-
void
1084+
int
10811085
pqSaveParameterStatus(PGconn*conn,constchar*name,constchar*value)
10821086
{
10831087
pgParameterStatus*pstatus;
@@ -1119,6 +1123,11 @@ pqSaveParameterStatus(PGconn *conn, const char *name, const char *value)
11191123
pstatus->next=conn->pstatus;
11201124
conn->pstatus=pstatus;
11211125
}
1126+
else
1127+
{
1128+
/* out of memory */
1129+
return0;
1130+
}
11221131

11231132
/*
11241133
* Save values of settings that are of interest to libpq in fields of the
@@ -1190,6 +1199,8 @@ pqSaveParameterStatus(PGconn *conn, const char *name, const char *value)
11901199
{
11911200
conn->scram_sha_256_iterations=atoi(value);
11921201
}
1202+
1203+
return1;
11931204
}
11941205

11951206

‎src/interfaces/libpq/fe-protocol3.c‎

Lines changed: 76 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
(id) == PqMsg_RowDescription)
4444

4545

46+
staticvoidhandleFatalError(PGconn*conn);
4647
staticvoidhandleSyncLoss(PGconn*conn,charid,intmsgLength);
4748
staticintgetRowDescriptions(PGconn*conn,intmsgLength);
4849
staticintgetParamDescriptions(PGconn*conn,intmsgLength);
@@ -120,12 +121,12 @@ pqParseInput3(PGconn *conn)
120121
conn))
121122
{
122123
/*
123-
*XXX add some better recovery code... plan is to skip over
124-
*the message using its length, then report an error. For the
125-
*moment, just treat this like loss of sync (which indeed it
126-
*might be!)
124+
*Abandon the connection. There's not much else we can
125+
*safely do; we can't just ignore the message or we could
126+
*miss important changes to the connection state.
127+
*pqCheckInBufferSpace() already reported the error.
127128
*/
128-
handleSyncLoss(conn,id,msgLength);
129+
handleFatalError(conn);
129130
}
130131
return;
131132
}
@@ -456,6 +457,11 @@ pqParseInput3(PGconn *conn)
456457
/* Normal case: parsing agrees with specified length */
457458
pqParseDone(conn,conn->inCursor);
458459
}
460+
elseif (conn->error_result&&conn->status==CONNECTION_BAD)
461+
{
462+
/* The connection was abandoned and we already reported it */
463+
return;
464+
}
459465
else
460466
{
461467
/* Trouble --- report it */
@@ -470,15 +476,14 @@ pqParseInput3(PGconn *conn)
470476
}
471477

472478
/*
473-
*handleSyncLoss: clean up afterloss of message-boundary sync
479+
*handleFatalError: clean up aftera nonrecoverable error
474480
*
475-
* There isn't really a lot we can do here except abandon the connection.
481+
* This is for errors where we need to abandon the connection. The caller has
482+
* already saved the error message in conn->errorMessage.
476483
*/
477484
staticvoid
478-
handleSyncLoss(PGconn*conn,charid,intmsgLength)
485+
handleFatalError(PGconn*conn)
479486
{
480-
libpq_append_conn_error(conn,"lost synchronization with server: got message type \"%c\", length %d",
481-
id,msgLength);
482487
/* build an error result holding the error message */
483488
pqSaveErrorResult(conn);
484489
conn->asyncStatus=PGASYNC_READY;/* drop out of PQgetResult wait loop */
@@ -487,6 +492,19 @@ handleSyncLoss(PGconn *conn, char id, int msgLength)
487492
conn->status=CONNECTION_BAD;/* No more connection to backend */
488493
}
489494

495+
/*
496+
* handleSyncLoss: clean up after loss of message-boundary sync
497+
*
498+
* There isn't really a lot we can do here except abandon the connection.
499+
*/
500+
staticvoid
501+
handleSyncLoss(PGconn*conn,charid,intmsgLength)
502+
{
503+
libpq_append_conn_error(conn,"lost synchronization with server: got message type \"%c\", length %d",
504+
id,msgLength);
505+
handleFatalError(conn);
506+
}
507+
490508
/*
491509
* parseInput subroutine to read a 'T' (row descriptions) message.
492510
* We'll build a new PGresult structure (unless called for a Describe
@@ -1519,7 +1537,11 @@ getParameterStatus(PGconn *conn)
15191537
returnEOF;
15201538
}
15211539
/* And save it */
1522-
pqSaveParameterStatus(conn,conn->workBuffer.data,valueBuf.data);
1540+
if (!pqSaveParameterStatus(conn,conn->workBuffer.data,valueBuf.data))
1541+
{
1542+
libpq_append_conn_error(conn,"out of memory");
1543+
handleFatalError(conn);
1544+
}
15231545
termPQExpBuffer(&valueBuf);
15241546
return0;
15251547
}
@@ -1551,8 +1573,8 @@ getBackendKeyData(PGconn *conn, int msgLength)
15511573
if (conn->be_cancel_key==NULL)
15521574
{
15531575
libpq_append_conn_error(conn,"out of memory");
1554-
/* discard the message */
1555-
returnEOF;
1576+
handleFatalError(conn);
1577+
return0;
15561578
}
15571579
if (pqGetnchar(conn->be_cancel_key,cancel_key_len,conn))
15581580
{
@@ -1589,10 +1611,21 @@ getNotify(PGconn *conn)
15891611
/* must save name while getting extra string */
15901612
svname=strdup(conn->workBuffer.data);
15911613
if (!svname)
1592-
returnEOF;
1614+
{
1615+
/*
1616+
* Notify messages can arrive at any state, so we cannot associate the
1617+
* error with any particular query. There's no way to return back an
1618+
* "async error", so the best we can do is drop the connection. That
1619+
* seems better than silently ignoring the notification.
1620+
*/
1621+
libpq_append_conn_error(conn,"out of memory");
1622+
handleFatalError(conn);
1623+
return0;
1624+
}
15931625
if (pqGets(&conn->workBuffer,conn))
15941626
{
1595-
free(svname);
1627+
if (svname)
1628+
free(svname);
15961629
returnEOF;
15971630
}
15981631

@@ -1604,21 +1637,26 @@ getNotify(PGconn *conn)
16041637
nmlen=strlen(svname);
16051638
extralen=strlen(conn->workBuffer.data);
16061639
newNotify= (PGnotify*)malloc(sizeof(PGnotify)+nmlen+extralen+2);
1607-
if (newNotify)
1608-
{
1609-
newNotify->relname= (char*)newNotify+sizeof(PGnotify);
1610-
strcpy(newNotify->relname,svname);
1611-
newNotify->extra=newNotify->relname+nmlen+1;
1612-
strcpy(newNotify->extra,conn->workBuffer.data);
1613-
newNotify->be_pid=be_pid;
1614-
newNotify->next=NULL;
1615-
if (conn->notifyTail)
1616-
conn->notifyTail->next=newNotify;
1617-
else
1618-
conn->notifyHead=newNotify;
1619-
conn->notifyTail=newNotify;
1640+
if (!newNotify)
1641+
{
1642+
free(svname);
1643+
libpq_append_conn_error(conn,"out of memory");
1644+
handleFatalError(conn);
1645+
return0;
16201646
}
16211647

1648+
newNotify->relname= (char*)newNotify+sizeof(PGnotify);
1649+
strcpy(newNotify->relname,svname);
1650+
newNotify->extra=newNotify->relname+nmlen+1;
1651+
strcpy(newNotify->extra,conn->workBuffer.data);
1652+
newNotify->be_pid=be_pid;
1653+
newNotify->next=NULL;
1654+
if (conn->notifyTail)
1655+
conn->notifyTail->next=newNotify;
1656+
else
1657+
conn->notifyHead=newNotify;
1658+
conn->notifyTail=newNotify;
1659+
16221660
free(svname);
16231661
return0;
16241662
}
@@ -1752,12 +1790,12 @@ getCopyDataMessage(PGconn *conn)
17521790
conn))
17531791
{
17541792
/*
1755-
*XXX add some better recovery code... plan is to skip over
1756-
*the message using its length, then report an error. For the
1757-
*moment, just treat this like loss of sync (which indeed it
1758-
*might be!)
1793+
*Abandon the connection. There's not much else we can
1794+
*safely do; we can't just ignore the message or we could
1795+
*miss important changes to the connection state.
1796+
*pqCheckInBufferSpace() already reported the error.
17591797
*/
1760-
handleSyncLoss(conn,id,msgLength);
1798+
handleFatalError(conn);
17611799
return-2;
17621800
}
17631801
return0;
@@ -2186,12 +2224,12 @@ pqFunctionCall3(PGconn *conn, Oid fnid,
21862224
conn))
21872225
{
21882226
/*
2189-
*XXX add some better recovery code... plan is to skip over
2190-
*the message using its length, then report an error. For the
2191-
*moment, just treat this like loss of sync (which indeed it
2192-
*might be!)
2227+
*Abandon the connection. There's not much else we can
2228+
*safely do; we can't just ignore the message or we could
2229+
*miss important changes to the connection state.
2230+
*pqCheckInBufferSpace() already reported the error.
21932231
*/
2194-
handleSyncLoss(conn,id,msgLength);
2232+
handleFatalError(conn);
21952233
break;
21962234
}
21972235
continue;

‎src/interfaces/libpq/libpq-int.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -746,7 +746,7 @@ extern PGresult *pqPrepareAsyncResult(PGconn *conn);
746746
externvoidpqInternalNotice(constPGNoticeHooks*hooks,constchar*fmt,...)pg_attribute_printf(2,3);
747747
externvoidpqSaveMessageField(PGresult*res,charcode,
748748
constchar*value);
749-
externvoidpqSaveParameterStatus(PGconn*conn,constchar*name,
749+
externintpqSaveParameterStatus(PGconn*conn,constchar*name,
750750
constchar*value);
751751
externintpqRowProcessor(PGconn*conn,constchar**errmsgp);
752752
externvoidpqCommandQueueAdvance(PGconn*conn,boolisReadyForQuery,

0 commit comments

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