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

Commit09c9ae8

Browse files
committed
Avoid resource leaks when a dblink connection fails.
If we hit out-of-memory between creating the PGconn and insertingit into dblink's hashtable, we'd lose track of the PGconn, whichis quite bad since it represents a live connection to a remote DB.Fix by rearranging things so that we create the hashtable entryfirst.Also reduce the number of states we have to deal with by getting ridof the separately-allocated remoteConn object, instead allocating itin-line in the hashtable entries. (That incidentally removes asession-lifespan memory leak observed in the regression tests.)There is an apparently-irreducible remaining OOM hazard, whichis that if the connection fails at the libpq level (ie it'sCONNECTION_BAD) then we have to pstrdup the PGconn's error messagebefore we can release it, and theoretically that could fail. However,in such cases we're only leaking memory not a live remote connection,so I'm not convinced that it's worth sweating over.This is a pretty low-probability failure mode of course, but losinga live connection seems bad enough to justify back-patching.Author: Tom Lane <tgl@sss.pgh.pa.us>Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com>Discussion:https://postgr.es/m/1346940.1748381911@sss.pgh.pa.usBackpatch-through: 13
1 parent130300a commit09c9ae8

File tree

1 file changed

+38
-38
lines changed

1 file changed

+38
-38
lines changed

‎contrib/dblink/dblink.c

Lines changed: 38 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ static PGresult *storeQueryResult(volatile storeInfo *sinfo, PGconn *conn, const
9999
staticvoidstoreRow(volatilestoreInfo*sinfo,PGresult*res,boolfirst);
100100
staticremoteConn*getConnectionByName(constchar*name);
101101
staticHTAB*createConnHash(void);
102-
staticvoidcreateNewConnection(constchar*name,remoteConn*rconn);
102+
staticremoteConn*createNewConnection(constchar*name);
103103
staticvoiddeleteConnection(constchar*name);
104104
staticchar**get_pkey_attnames(Relationrel,int16*indnkeyatts);
105105
staticchar**get_text_array_contents(ArrayType*array,int*numitems);
@@ -112,7 +112,7 @@ static HeapTuple get_tuple_of_interest(Relation rel, int *pkattnums, int pknumat
112112
staticRelationget_rel_from_relname(text*relname_text,LOCKMODElockmode,AclModeaclmode);
113113
staticchar*generate_relation_name(Relationrel);
114114
staticvoiddblink_connstr_check(constchar*connstr);
115-
staticvoiddblink_security_check(PGconn*conn,remoteConn*rconn);
115+
staticvoiddblink_security_check(PGconn*conn,constchar*connname);
116116
staticvoiddblink_res_error(PGconn*conn,constchar*conname,PGresult*res,
117117
boolfail,constchar*fmt,...)pg_attribute_printf(5,6);
118118
staticchar*get_connect_string(constchar*servername);
@@ -130,16 +130,22 @@ static remoteConn *pconn = NULL;
130130
staticHTAB*remoteConnHash=NULL;
131131

132132
/*
133-
*Following islist that holds multiple remote connections.
133+
*Following ishash that holds multiple remote connections.
134134
*Calling convention of each dblink function changes to accept
135-
*connection name as the first parameter. The connectionlist is
135+
*connection name as the first parameter. The connectionhash is
136136
*much like ecpg e.g. a mapping between a name and a PGconn object.
137+
*
138+
*To avoid potentially leaking a PGconn object in case of out-of-memory
139+
*errors, we first create the hash entry, then open the PGconn.
140+
*Hence, a hash entry whose rconn.conn pointer is NULL must be
141+
*understood as a leftover from a failed create; it should be ignored
142+
*by lookup operations, and silently replaced by create operations.
137143
*/
138144

139145
typedefstructremoteConnHashEnt
140146
{
141147
charname[NAMEDATALEN];
142-
remoteConn*rconn;
148+
remoteConnrconn;
143149
}remoteConnHashEnt;
144150

145151
/* initial number of connection hashes */
@@ -238,7 +244,7 @@ dblink_get_conn(char *conname_or_str,
238244
errmsg("could not establish connection"),
239245
errdetail_internal("%s",msg)));
240246
}
241-
dblink_security_check(conn,rconn);
247+
dblink_security_check(conn,NULL);
242248
if (PQclientEncoding(conn)!=GetDatabaseEncoding())
243249
PQsetClientEncoding(conn,GetDatabaseEncodingName());
244250
freeconn= true;
@@ -298,15 +304,6 @@ dblink_connect(PG_FUNCTION_ARGS)
298304
elseif (PG_NARGS()==1)
299305
conname_or_str=text_to_cstring(PG_GETARG_TEXT_PP(0));
300306

301-
if (connname)
302-
{
303-
rconn= (remoteConn*)MemoryContextAlloc(TopMemoryContext,
304-
sizeof(remoteConn));
305-
rconn->conn=NULL;
306-
rconn->openCursorCount=0;
307-
rconn->newXactForCursor= false;
308-
}
309-
310307
/* first check for valid foreign data server */
311308
connstr=get_connect_string(conname_or_str);
312309
if (connstr==NULL)
@@ -337,6 +334,13 @@ dblink_connect(PG_FUNCTION_ARGS)
337334
#endif
338335
}
339336

337+
/* if we need a hashtable entry, make that first, since it might fail */
338+
if (connname)
339+
{
340+
rconn=createNewConnection(connname);
341+
Assert(rconn->conn==NULL);
342+
}
343+
340344
/* OK to make connection */
341345
conn=PQconnectdb(connstr);
342346

@@ -345,8 +349,8 @@ dblink_connect(PG_FUNCTION_ARGS)
345349
msg=pchomp(PQerrorMessage(conn));
346350
PQfinish(conn);
347351
ReleaseExternalFD();
348-
if (rconn)
349-
pfree(rconn);
352+
if (connname)
353+
deleteConnection(connname);
350354

351355
ereport(ERROR,
352356
(errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
@@ -355,16 +359,16 @@ dblink_connect(PG_FUNCTION_ARGS)
355359
}
356360

357361
/* check password actually used if not superuser */
358-
dblink_security_check(conn,rconn);
362+
dblink_security_check(conn,connname);
359363

360364
/* attempt to set client encoding to match server encoding, if needed */
361365
if (PQclientEncoding(conn)!=GetDatabaseEncoding())
362366
PQsetClientEncoding(conn,GetDatabaseEncodingName());
363367

368+
/* all OK, save away the conn */
364369
if (connname)
365370
{
366371
rconn->conn=conn;
367-
createNewConnection(connname,rconn);
368372
}
369373
else
370374
{
@@ -408,10 +412,7 @@ dblink_disconnect(PG_FUNCTION_ARGS)
408412
PQfinish(conn);
409413
ReleaseExternalFD();
410414
if (rconn)
411-
{
412415
deleteConnection(conname);
413-
pfree(rconn);
414-
}
415416
else
416417
pconn->conn=NULL;
417418

@@ -1332,6 +1333,9 @@ dblink_get_connections(PG_FUNCTION_ARGS)
13321333
hash_seq_init(&status,remoteConnHash);
13331334
while ((hentry= (remoteConnHashEnt*)hash_seq_search(&status))!=NULL)
13341335
{
1336+
/* ignore it if it's not an open connection */
1337+
if (hentry->rconn.conn==NULL)
1338+
continue;
13351339
/* stash away current value */
13361340
astate=accumArrayResult(astate,
13371341
CStringGetTextDatum(hentry->name),
@@ -2570,8 +2574,8 @@ getConnectionByName(const char *name)
25702574
hentry= (remoteConnHashEnt*)hash_search(remoteConnHash,
25712575
key,HASH_FIND,NULL);
25722576

2573-
if (hentry)
2574-
returnhentry->rconn;
2577+
if (hentry&&hentry->rconn.conn!=NULL)
2578+
return&hentry->rconn;
25752579

25762580
returnNULL;
25772581
}
@@ -2588,8 +2592,8 @@ createConnHash(void)
25882592
HASH_ELEM |HASH_STRINGS);
25892593
}
25902594

2591-
staticvoid
2592-
createNewConnection(constchar*name,remoteConn*rconn)
2595+
staticremoteConn*
2596+
createNewConnection(constchar*name)
25932597
{
25942598
remoteConnHashEnt*hentry;
25952599
boolfound;
@@ -2603,19 +2607,15 @@ createNewConnection(const char *name, remoteConn *rconn)
26032607
hentry= (remoteConnHashEnt*)hash_search(remoteConnHash,key,
26042608
HASH_ENTER,&found);
26052609

2606-
if (found)
2607-
{
2608-
PQfinish(rconn->conn);
2609-
ReleaseExternalFD();
2610-
pfree(rconn);
2611-
2610+
if (found&&hentry->rconn.conn!=NULL)
26122611
ereport(ERROR,
26132612
(errcode(ERRCODE_DUPLICATE_OBJECT),
26142613
errmsg("duplicate connection name")));
2615-
}
26162614

2617-
hentry->rconn=rconn;
2618-
strlcpy(hentry->name,name,sizeof(hentry->name));
2615+
/* New, or reusable, so initialize the rconn struct to zeroes */
2616+
memset(&hentry->rconn,0,sizeof(remoteConn));
2617+
2618+
return&hentry->rconn;
26192619
}
26202620

26212621
staticvoid
@@ -2640,16 +2640,16 @@ deleteConnection(const char *name)
26402640
}
26412641

26422642
staticvoid
2643-
dblink_security_check(PGconn*conn,remoteConn*rconn)
2643+
dblink_security_check(PGconn*conn,constchar*connname)
26442644
{
26452645
if (!superuser())
26462646
{
26472647
if (!PQconnectionUsedPassword(conn))
26482648
{
26492649
PQfinish(conn);
26502650
ReleaseExternalFD();
2651-
if (rconn)
2652-
pfree(rconn);
2651+
if (connname)
2652+
deleteConnection(connname);
26532653

26542654
ereport(ERROR,
26552655
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),

0 commit comments

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