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

Commite20b325

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 parentb64c585 commite20b325

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
@@ -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);
@@ -114,7 +114,8 @@ static Relation get_rel_from_relname(text *relname_text, LOCKMODE lockmode, AclM
114114
staticchar*generate_relation_name(Relationrel);
115115
staticvoiddblink_connstr_check(constchar*connstr);
116116
staticbooldblink_connstr_has_pw(constchar*connstr);
117-
staticvoiddblink_security_check(PGconn*conn,remoteConn*rconn,constchar*connstr);
117+
staticvoiddblink_security_check(PGconn*conn,constchar*connname,
118+
constchar*connstr);
118119
staticvoiddblink_res_error(PGconn*conn,constchar*conname,PGresult*res,
119120
boolfail,constchar*fmt,...)pg_attribute_printf(5,6);
120121
staticchar*get_connect_string(constchar*servername);
@@ -137,16 +138,22 @@ static uint32 dblink_we_get_conn = 0;
137138
staticuint32dblink_we_get_result=0;
138139

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

146153
typedefstructremoteConnHashEnt
147154
{
148155
charname[NAMEDATALEN];
149-
remoteConn*rconn;
156+
remoteConnrconn;
150157
}remoteConnHashEnt;
151158

152159
/* initial number of connection hashes */
@@ -225,7 +232,7 @@ dblink_get_conn(char *conname_or_str,
225232
errmsg("could not establish connection"),
226233
errdetail_internal("%s",msg)));
227234
}
228-
dblink_security_check(conn,rconn,connstr);
235+
dblink_security_check(conn,NULL,connstr);
229236
if (PQclientEncoding(conn)!=GetDatabaseEncoding())
230237
PQsetClientEncoding(conn,GetDatabaseEncodingName());
231238
freeconn= true;
@@ -288,15 +295,6 @@ dblink_connect(PG_FUNCTION_ARGS)
288295
elseif (PG_NARGS()==1)
289296
conname_or_str=text_to_cstring(PG_GETARG_TEXT_PP(0));
290297

291-
if (connname)
292-
{
293-
rconn= (remoteConn*)MemoryContextAlloc(TopMemoryContext,
294-
sizeof(remoteConn));
295-
rconn->conn=NULL;
296-
rconn->openCursorCount=0;
297-
rconn->newXactForCursor= false;
298-
}
299-
300298
/* first check for valid foreign data server */
301299
connstr=get_connect_string(conname_or_str);
302300
if (connstr==NULL)
@@ -309,15 +307,22 @@ dblink_connect(PG_FUNCTION_ARGS)
309307
if (dblink_we_connect==0)
310308
dblink_we_connect=WaitEventExtensionNew("DblinkConnect");
311309

310+
/* if we need a hashtable entry, make that first, since it might fail */
311+
if (connname)
312+
{
313+
rconn=createNewConnection(connname);
314+
Assert(rconn->conn==NULL);
315+
}
316+
312317
/* OK to make connection */
313318
conn=libpqsrv_connect(connstr,dblink_we_connect);
314319

315320
if (PQstatus(conn)==CONNECTION_BAD)
316321
{
317322
msg=pchomp(PQerrorMessage(conn));
318323
libpqsrv_disconnect(conn);
319-
if (rconn)
320-
pfree(rconn);
324+
if (connname)
325+
deleteConnection(connname);
321326

322327
ereport(ERROR,
323328
(errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
@@ -326,16 +331,16 @@ dblink_connect(PG_FUNCTION_ARGS)
326331
}
327332

328333
/* check password actually used if not superuser */
329-
dblink_security_check(conn,rconn,connstr);
334+
dblink_security_check(conn,connname,connstr);
330335

331336
/* attempt to set client encoding to match server encoding, if needed */
332337
if (PQclientEncoding(conn)!=GetDatabaseEncoding())
333338
PQsetClientEncoding(conn,GetDatabaseEncodingName());
334339

340+
/* all OK, save away the conn */
335341
if (connname)
336342
{
337343
rconn->conn=conn;
338-
createNewConnection(connname,rconn);
339344
}
340345
else
341346
{
@@ -375,10 +380,7 @@ dblink_disconnect(PG_FUNCTION_ARGS)
375380

376381
libpqsrv_disconnect(conn);
377382
if (rconn)
378-
{
379383
deleteConnection(conname);
380-
pfree(rconn);
381-
}
382384
else
383385
pconn->conn=NULL;
384386

@@ -1296,6 +1298,9 @@ dblink_get_connections(PG_FUNCTION_ARGS)
12961298
hash_seq_init(&status,remoteConnHash);
12971299
while ((hentry= (remoteConnHashEnt*)hash_seq_search(&status))!=NULL)
12981300
{
1301+
/* ignore it if it's not an open connection */
1302+
if (hentry->rconn.conn==NULL)
1303+
continue;
12991304
/* stash away current value */
13001305
astate=accumArrayResult(astate,
13011306
CStringGetTextDatum(hentry->name),
@@ -2533,8 +2538,8 @@ getConnectionByName(const char *name)
25332538
hentry= (remoteConnHashEnt*)hash_search(remoteConnHash,
25342539
key,HASH_FIND,NULL);
25352540

2536-
if (hentry)
2537-
returnhentry->rconn;
2541+
if (hentry&&hentry->rconn.conn!=NULL)
2542+
return&hentry->rconn;
25382543

25392544
returnNULL;
25402545
}
@@ -2551,8 +2556,8 @@ createConnHash(void)
25512556
HASH_ELEM |HASH_STRINGS);
25522557
}
25532558

2554-
staticvoid
2555-
createNewConnection(constchar*name,remoteConn*rconn)
2559+
staticremoteConn*
2560+
createNewConnection(constchar*name)
25562561
{
25572562
remoteConnHashEnt*hentry;
25582563
boolfound;
@@ -2566,17 +2571,15 @@ createNewConnection(const char *name, remoteConn *rconn)
25662571
hentry= (remoteConnHashEnt*)hash_search(remoteConnHash,key,
25672572
HASH_ENTER,&found);
25682573

2569-
if (found)
2570-
{
2571-
libpqsrv_disconnect(rconn->conn);
2572-
pfree(rconn);
2573-
2574+
if (found&&hentry->rconn.conn!=NULL)
25742575
ereport(ERROR,
25752576
(errcode(ERRCODE_DUPLICATE_OBJECT),
25762577
errmsg("duplicate connection name")));
2577-
}
25782578

2579-
hentry->rconn=rconn;
2579+
/* New, or reusable, so initialize the rconn struct to zeroes */
2580+
memset(&hentry->rconn,0,sizeof(remoteConn));
2581+
2582+
return&hentry->rconn;
25802583
}
25812584

25822585
staticvoid
@@ -2604,9 +2607,12 @@ deleteConnection(const char *name)
26042607
* We need to make sure that the connection made used credentials
26052608
* which were provided by the user, so check what credentials were
26062609
* used to connect and then make sure that they came from the user.
2610+
*
2611+
* On failure, we close "conn" and also delete the hashtable entry
2612+
* identified by "connname" (if that's not NULL).
26072613
*/
26082614
staticvoid
2609-
dblink_security_check(PGconn*conn,remoteConn*rconn,constchar*connstr)
2615+
dblink_security_check(PGconn*conn,constchar*connname,constchar*connstr)
26102616
{
26112617
/* Superuser bypasses security check */
26122618
if (superuser())
@@ -2624,8 +2630,8 @@ dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr)
26242630

26252631
/* Otherwise, fail out */
26262632
libpqsrv_disconnect(conn);
2627-
if (rconn)
2628-
pfree(rconn);
2633+
if (connname)
2634+
deleteConnection(connname);
26292635

26302636
ereport(ERROR,
26312637
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),

0 commit comments

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