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

Commit470273d

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 parent3c4d755 commit470273d

File tree

1 file changed

+42
-36
lines changed

1 file changed

+42
-36
lines changed

‎contrib/dblink/dblink.c

Lines changed: 42 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ static PGresult *storeQueryResult(volatile storeInfo *sinfo, PGconn *conn, const
105105
staticvoidstoreRow(volatilestoreInfo*sinfo,PGresult*res,boolfirst);
106106
staticremoteConn*getConnectionByName(constchar*name);
107107
staticHTAB*createConnHash(void);
108-
staticvoidcreateNewConnection(constchar*name,remoteConn*rconn);
108+
staticremoteConn*createNewConnection(constchar*name);
109109
staticvoiddeleteConnection(constchar*name);
110110
staticchar**get_pkey_attnames(Relationrel,int16*indnkeyatts);
111111
staticchar**get_text_array_contents(ArrayType*array,int*numitems);
@@ -119,7 +119,8 @@ static Relation get_rel_from_relname(text *relname_text, LOCKMODE lockmode, AclM
119119
staticchar*generate_relation_name(Relationrel);
120120
staticvoiddblink_connstr_check(constchar*connstr);
121121
staticbooldblink_connstr_has_pw(constchar*connstr);
122-
staticvoiddblink_security_check(PGconn*conn,remoteConn*rconn,constchar*connstr);
122+
staticvoiddblink_security_check(PGconn*conn,constchar*connname,
123+
constchar*connstr);
123124
staticvoiddblink_res_error(PGconn*conn,constchar*conname,PGresult*res,
124125
boolfail,constchar*fmt,...)pg_attribute_printf(5,6);
125126
staticchar*get_connect_string(constchar*servername);
@@ -147,16 +148,22 @@ static uint32 dblink_we_get_conn = 0;
147148
staticuint32dblink_we_get_result=0;
148149

149150
/*
150-
*Following islist that holds multiple remote connections.
151+
*Following ishash that holds multiple remote connections.
151152
*Calling convention of each dblink function changes to accept
152-
*connection name as the first parameter. The connectionlist is
153+
*connection name as the first parameter. The connectionhash is
153154
*much like ecpg e.g. a mapping between a name and a PGconn object.
155+
*
156+
*To avoid potentially leaking a PGconn object in case of out-of-memory
157+
*errors, we first create the hash entry, then open the PGconn.
158+
*Hence, a hash entry whose rconn.conn pointer is NULL must be
159+
*understood as a leftover from a failed create; it should be ignored
160+
*by lookup operations, and silently replaced by create operations.
154161
*/
155162

156163
typedefstructremoteConnHashEnt
157164
{
158165
charname[NAMEDATALEN];
159-
remoteConn*rconn;
166+
remoteConnrconn;
160167
}remoteConnHashEnt;
161168

162169
/* initial number of connection hashes */
@@ -233,7 +240,7 @@ dblink_get_conn(char *conname_or_str,
233240
errmsg("could not establish connection"),
234241
errdetail_internal("%s",msg)));
235242
}
236-
dblink_security_check(conn,rconn,connstr);
243+
dblink_security_check(conn,NULL,connstr);
237244
if (PQclientEncoding(conn)!=GetDatabaseEncoding())
238245
PQsetClientEncoding(conn,GetDatabaseEncodingName());
239246
freeconn= true;
@@ -296,15 +303,6 @@ dblink_connect(PG_FUNCTION_ARGS)
296303
elseif (PG_NARGS()==1)
297304
conname_or_str=text_to_cstring(PG_GETARG_TEXT_PP(0));
298305

299-
if (connname)
300-
{
301-
rconn= (remoteConn*)MemoryContextAlloc(TopMemoryContext,
302-
sizeof(remoteConn));
303-
rconn->conn=NULL;
304-
rconn->openCursorCount=0;
305-
rconn->newXactForCursor= false;
306-
}
307-
308306
/* first check for valid foreign data server */
309307
connstr=get_connect_string(conname_or_str);
310308
if (connstr==NULL)
@@ -317,15 +315,22 @@ dblink_connect(PG_FUNCTION_ARGS)
317315
if (dblink_we_connect==0)
318316
dblink_we_connect=WaitEventExtensionNew("DblinkConnect");
319317

318+
/* if we need a hashtable entry, make that first, since it might fail */
319+
if (connname)
320+
{
321+
rconn=createNewConnection(connname);
322+
Assert(rconn->conn==NULL);
323+
}
324+
320325
/* OK to make connection */
321326
conn=libpqsrv_connect(connstr,dblink_we_connect);
322327

323328
if (PQstatus(conn)==CONNECTION_BAD)
324329
{
325330
msg=pchomp(PQerrorMessage(conn));
326331
libpqsrv_disconnect(conn);
327-
if (rconn)
328-
pfree(rconn);
332+
if (connname)
333+
deleteConnection(connname);
329334

330335
ereport(ERROR,
331336
(errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
@@ -334,16 +339,16 @@ dblink_connect(PG_FUNCTION_ARGS)
334339
}
335340

336341
/* check password actually used if not superuser */
337-
dblink_security_check(conn,rconn,connstr);
342+
dblink_security_check(conn,connname,connstr);
338343

339344
/* attempt to set client encoding to match server encoding, if needed */
340345
if (PQclientEncoding(conn)!=GetDatabaseEncoding())
341346
PQsetClientEncoding(conn,GetDatabaseEncodingName());
342347

348+
/* all OK, save away the conn */
343349
if (connname)
344350
{
345351
rconn->conn=conn;
346-
createNewConnection(connname,rconn);
347352
}
348353
else
349354
{
@@ -383,10 +388,7 @@ dblink_disconnect(PG_FUNCTION_ARGS)
383388

384389
libpqsrv_disconnect(conn);
385390
if (rconn)
386-
{
387391
deleteConnection(conname);
388-
pfree(rconn);
389-
}
390392
else
391393
pconn->conn=NULL;
392394

@@ -1304,6 +1306,9 @@ dblink_get_connections(PG_FUNCTION_ARGS)
13041306
hash_seq_init(&status,remoteConnHash);
13051307
while ((hentry= (remoteConnHashEnt*)hash_seq_search(&status))!=NULL)
13061308
{
1309+
/* ignore it if it's not an open connection */
1310+
if (hentry->rconn.conn==NULL)
1311+
continue;
13071312
/* stash away current value */
13081313
astate=accumArrayResult(astate,
13091314
CStringGetTextDatum(hentry->name),
@@ -2539,8 +2544,8 @@ getConnectionByName(const char *name)
25392544
hentry= (remoteConnHashEnt*)hash_search(remoteConnHash,
25402545
key,HASH_FIND,NULL);
25412546

2542-
if (hentry)
2543-
returnhentry->rconn;
2547+
if (hentry&&hentry->rconn.conn!=NULL)
2548+
return&hentry->rconn;
25442549

25452550
returnNULL;
25462551
}
@@ -2557,8 +2562,8 @@ createConnHash(void)
25572562
HASH_ELEM |HASH_STRINGS);
25582563
}
25592564

2560-
staticvoid
2561-
createNewConnection(constchar*name,remoteConn*rconn)
2565+
staticremoteConn*
2566+
createNewConnection(constchar*name)
25622567
{
25632568
remoteConnHashEnt*hentry;
25642569
boolfound;
@@ -2572,17 +2577,15 @@ createNewConnection(const char *name, remoteConn *rconn)
25722577
hentry= (remoteConnHashEnt*)hash_search(remoteConnHash,key,
25732578
HASH_ENTER,&found);
25742579

2575-
if (found)
2576-
{
2577-
libpqsrv_disconnect(rconn->conn);
2578-
pfree(rconn);
2579-
2580+
if (found&&hentry->rconn.conn!=NULL)
25802581
ereport(ERROR,
25812582
(errcode(ERRCODE_DUPLICATE_OBJECT),
25822583
errmsg("duplicate connection name")));
2583-
}
25842584

2585-
hentry->rconn=rconn;
2585+
/* New, or reusable, so initialize the rconn struct to zeroes */
2586+
memset(&hentry->rconn,0,sizeof(remoteConn));
2587+
2588+
return&hentry->rconn;
25862589
}
25872590

25882591
staticvoid
@@ -2671,9 +2674,12 @@ dblink_connstr_has_required_scram_options(const char *connstr)
26712674
* We need to make sure that the connection made used credentials
26722675
* which were provided by the user, so check what credentials were
26732676
* used to connect and then make sure that they came from the user.
2677+
*
2678+
* On failure, we close "conn" and also delete the hashtable entry
2679+
* identified by "connname" (if that's not NULL).
26742680
*/
26752681
staticvoid
2676-
dblink_security_check(PGconn*conn,remoteConn*rconn,constchar*connstr)
2682+
dblink_security_check(PGconn*conn,constchar*connname,constchar*connstr)
26772683
{
26782684
/* Superuser bypasses security check */
26792685
if (superuser())
@@ -2703,8 +2709,8 @@ dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr)
27032709

27042710
/* Otherwise, fail out */
27052711
libpqsrv_disconnect(conn);
2706-
if (rconn)
2707-
pfree(rconn);
2712+
if (connname)
2713+
deleteConnection(connname);
27082714

27092715
ereport(ERROR,
27102716
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),

0 commit comments

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