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

Commite7d3d4e

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 parent3c03b8c commite7d3d4e

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
@@ -100,7 +100,7 @@ static PGresult *storeQueryResult(volatile storeInfo *sinfo, PGconn *conn, const
100100
staticvoidstoreRow(volatilestoreInfo*sinfo,PGresult*res,boolfirst);
101101
staticremoteConn*getConnectionByName(constchar*name);
102102
staticHTAB*createConnHash(void);
103-
staticvoidcreateNewConnection(constchar*name,remoteConn*rconn);
103+
staticremoteConn*createNewConnection(constchar*name);
104104
staticvoiddeleteConnection(constchar*name);
105105
staticchar**get_pkey_attnames(Relationrel,int16*indnkeyatts);
106106
staticchar**get_text_array_contents(ArrayType*array,int*numitems);
@@ -113,7 +113,7 @@ static HeapTuple get_tuple_of_interest(Relation rel, int *pkattnums, int pknumat
113113
staticRelationget_rel_from_relname(text*relname_text,LOCKMODElockmode,AclModeaclmode);
114114
staticchar*generate_relation_name(Relationrel);
115115
staticvoiddblink_connstr_check(constchar*connstr);
116-
staticvoiddblink_security_check(PGconn*conn,remoteConn*rconn);
116+
staticvoiddblink_security_check(PGconn*conn,constchar*connname);
117117
staticvoiddblink_res_error(PGconn*conn,constchar*conname,PGresult*res,
118118
boolfail,constchar*fmt,...)pg_attribute_printf(5,6);
119119
staticchar*get_connect_string(constchar*servername);
@@ -131,16 +131,22 @@ static remoteConn *pconn = NULL;
131131
staticHTAB*remoteConnHash=NULL;
132132

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

140146
typedefstructremoteConnHashEnt
141147
{
142148
charname[NAMEDATALEN];
143-
remoteConn*rconn;
149+
remoteConnrconn;
144150
}remoteConnHashEnt;
145151

146152
/* initial number of connection hashes */
@@ -239,7 +245,7 @@ dblink_get_conn(char *conname_or_str,
239245
errmsg("could not establish connection"),
240246
errdetail_internal("%s",msg)));
241247
}
242-
dblink_security_check(conn,rconn);
248+
dblink_security_check(conn,NULL);
243249
if (PQclientEncoding(conn)!=GetDatabaseEncoding())
244250
PQsetClientEncoding(conn,GetDatabaseEncodingName());
245251
freeconn= true;
@@ -299,15 +305,6 @@ dblink_connect(PG_FUNCTION_ARGS)
299305
elseif (PG_NARGS()==1)
300306
conname_or_str=text_to_cstring(PG_GETARG_TEXT_PP(0));
301307

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

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

@@ -346,8 +350,8 @@ dblink_connect(PG_FUNCTION_ARGS)
346350
msg=pchomp(PQerrorMessage(conn));
347351
PQfinish(conn);
348352
ReleaseExternalFD();
349-
if (rconn)
350-
pfree(rconn);
353+
if (connname)
354+
deleteConnection(connname);
351355

352356
ereport(ERROR,
353357
(errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
@@ -356,16 +360,16 @@ dblink_connect(PG_FUNCTION_ARGS)
356360
}
357361

358362
/* check password actually used if not superuser */
359-
dblink_security_check(conn,rconn);
363+
dblink_security_check(conn,connname);
360364

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

369+
/* all OK, save away the conn */
365370
if (connname)
366371
{
367372
rconn->conn=conn;
368-
createNewConnection(connname,rconn);
369373
}
370374
else
371375
{
@@ -409,10 +413,7 @@ dblink_disconnect(PG_FUNCTION_ARGS)
409413
PQfinish(conn);
410414
ReleaseExternalFD();
411415
if (rconn)
412-
{
413416
deleteConnection(conname);
414-
pfree(rconn);
415-
}
416417
else
417418
pconn->conn=NULL;
418419

@@ -1336,6 +1337,9 @@ dblink_get_connections(PG_FUNCTION_ARGS)
13361337
hash_seq_init(&status,remoteConnHash);
13371338
while ((hentry= (remoteConnHashEnt*)hash_seq_search(&status))!=NULL)
13381339
{
1340+
/* ignore it if it's not an open connection */
1341+
if (hentry->rconn.conn==NULL)
1342+
continue;
13391343
/* stash away current value */
13401344
astate=accumArrayResult(astate,
13411345
CStringGetTextDatum(hentry->name),
@@ -2597,8 +2601,8 @@ getConnectionByName(const char *name)
25972601
hentry= (remoteConnHashEnt*)hash_search(remoteConnHash,
25982602
key,HASH_FIND,NULL);
25992603

2600-
if (hentry)
2601-
returnhentry->rconn;
2604+
if (hentry&&hentry->rconn.conn!=NULL)
2605+
return&hentry->rconn;
26022606

26032607
returnNULL;
26042608
}
@@ -2614,8 +2618,8 @@ createConnHash(void)
26142618
returnhash_create("Remote Con hash",NUMCONN,&ctl,HASH_ELEM);
26152619
}
26162620

2617-
staticvoid
2618-
createNewConnection(constchar*name,remoteConn*rconn)
2621+
staticremoteConn*
2622+
createNewConnection(constchar*name)
26192623
{
26202624
remoteConnHashEnt*hentry;
26212625
boolfound;
@@ -2629,19 +2633,15 @@ createNewConnection(const char *name, remoteConn *rconn)
26292633
hentry= (remoteConnHashEnt*)hash_search(remoteConnHash,key,
26302634
HASH_ENTER,&found);
26312635

2632-
if (found)
2633-
{
2634-
PQfinish(rconn->conn);
2635-
ReleaseExternalFD();
2636-
pfree(rconn);
2637-
2636+
if (found&&hentry->rconn.conn!=NULL)
26382637
ereport(ERROR,
26392638
(errcode(ERRCODE_DUPLICATE_OBJECT),
26402639
errmsg("duplicate connection name")));
2641-
}
26422640

2643-
hentry->rconn=rconn;
2644-
strlcpy(hentry->name,name,sizeof(hentry->name));
2641+
/* New, or reusable, so initialize the rconn struct to zeroes */
2642+
memset(&hentry->rconn,0,sizeof(remoteConn));
2643+
2644+
return&hentry->rconn;
26452645
}
26462646

26472647
staticvoid
@@ -2667,16 +2667,16 @@ deleteConnection(const char *name)
26672667
}
26682668

26692669
staticvoid
2670-
dblink_security_check(PGconn*conn,remoteConn*rconn)
2670+
dblink_security_check(PGconn*conn,constchar*connname)
26712671
{
26722672
if (!superuser())
26732673
{
26742674
if (!PQconnectionUsedPassword(conn))
26752675
{
26762676
PQfinish(conn);
26772677
ReleaseExternalFD();
2678-
if (rconn)
2679-
pfree(rconn);
2678+
if (connname)
2679+
deleteConnection(connname);
26802680

26812681
ereport(ERROR,
26822682
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),

0 commit comments

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