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

Commit2cd2222

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 parent8a1459f commit2cd2222

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

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

2599-
if (hentry)
2600-
returnhentry->rconn;
2603+
if (hentry&&hentry->rconn.conn!=NULL)
2604+
return&hentry->rconn;
26012605

26022606
returnNULL;
26032607
}
@@ -2614,8 +2618,8 @@ createConnHash(void)
26142618
HASH_ELEM |HASH_STRINGS);
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