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

Commit8eef55d

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 parent7e8b44f commit8eef55d

File tree

1 file changed

+42
-37
lines changed

1 file changed

+42
-37
lines changed

‎contrib/dblink/dblink.c

Lines changed: 42 additions & 37 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);
@@ -132,16 +133,22 @@ static remoteConn *pconn = NULL;
132133
staticHTAB*remoteConnHash=NULL;
133134

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

141148
typedefstructremoteConnHashEnt
142149
{
143150
charname[NAMEDATALEN];
144-
remoteConn*rconn;
151+
remoteConnrconn;
145152
}remoteConnHashEnt;
146153

147154
/* initial number of connection hashes */
@@ -216,7 +223,7 @@ dblink_get_conn(char *conname_or_str,
216223
errmsg("could not establish connection"),
217224
errdetail_internal("%s",msg)));
218225
}
219-
dblink_security_check(conn,rconn,connstr);
226+
dblink_security_check(conn,NULL,connstr);
220227
if (PQclientEncoding(conn)!=GetDatabaseEncoding())
221228
PQsetClientEncoding(conn,GetDatabaseEncodingName());
222229
freeconn= true;
@@ -276,15 +283,6 @@ dblink_connect(PG_FUNCTION_ARGS)
276283
elseif (PG_NARGS()==1)
277284
conname_or_str=text_to_cstring(PG_GETARG_TEXT_PP(0));
278285

279-
if (connname)
280-
{
281-
rconn= (remoteConn*)MemoryContextAlloc(TopMemoryContext,
282-
sizeof(remoteConn));
283-
rconn->conn=NULL;
284-
rconn->openCursorCount=0;
285-
rconn->newXactForCursor= false;
286-
}
287-
288286
/* first check for valid foreign data server */
289287
connstr=get_connect_string(conname_or_str);
290288
if (connstr==NULL)
@@ -293,15 +291,22 @@ dblink_connect(PG_FUNCTION_ARGS)
293291
/* check password in connection string if not superuser */
294292
dblink_connstr_check(connstr);
295293

294+
/* if we need a hashtable entry, make that first, since it might fail */
295+
if (connname)
296+
{
297+
rconn=createNewConnection(connname);
298+
Assert(rconn->conn==NULL);
299+
}
300+
296301
/* OK to make connection */
297302
conn=libpqsrv_connect(connstr,PG_WAIT_EXTENSION);
298303

299304
if (PQstatus(conn)==CONNECTION_BAD)
300305
{
301306
msg=pchomp(PQerrorMessage(conn));
302307
libpqsrv_disconnect(conn);
303-
if (rconn)
304-
pfree(rconn);
308+
if (connname)
309+
deleteConnection(connname);
305310

306311
ereport(ERROR,
307312
(errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
@@ -310,16 +315,16 @@ dblink_connect(PG_FUNCTION_ARGS)
310315
}
311316

312317
/* check password actually used if not superuser */
313-
dblink_security_check(conn,rconn,connstr);
318+
dblink_security_check(conn,connname,connstr);
314319

315320
/* attempt to set client encoding to match server encoding, if needed */
316321
if (PQclientEncoding(conn)!=GetDatabaseEncoding())
317322
PQsetClientEncoding(conn,GetDatabaseEncodingName());
318323

324+
/* all OK, save away the conn */
319325
if (connname)
320326
{
321327
rconn->conn=conn;
322-
createNewConnection(connname,rconn);
323328
}
324329
else
325330
{
@@ -359,10 +364,7 @@ dblink_disconnect(PG_FUNCTION_ARGS)
359364

360365
libpqsrv_disconnect(conn);
361366
if (rconn)
362-
{
363367
deleteConnection(conname);
364-
pfree(rconn);
365-
}
366368
else
367369
pconn->conn=NULL;
368370

@@ -1280,6 +1282,9 @@ dblink_get_connections(PG_FUNCTION_ARGS)
12801282
hash_seq_init(&status,remoteConnHash);
12811283
while ((hentry= (remoteConnHashEnt*)hash_seq_search(&status))!=NULL)
12821284
{
1285+
/* ignore it if it's not an open connection */
1286+
if (hentry->rconn.conn==NULL)
1287+
continue;
12831288
/* stash away current value */
12841289
astate=accumArrayResult(astate,
12851290
CStringGetTextDatum(hentry->name),
@@ -2520,8 +2525,8 @@ getConnectionByName(const char *name)
25202525
hentry= (remoteConnHashEnt*)hash_search(remoteConnHash,
25212526
key,HASH_FIND,NULL);
25222527

2523-
if (hentry)
2524-
returnhentry->rconn;
2528+
if (hentry&&hentry->rconn.conn!=NULL)
2529+
return&hentry->rconn;
25252530

25262531
returnNULL;
25272532
}
@@ -2538,8 +2543,8 @@ createConnHash(void)
25382543
HASH_ELEM |HASH_STRINGS);
25392544
}
25402545

2541-
staticvoid
2542-
createNewConnection(constchar*name,remoteConn*rconn)
2546+
staticremoteConn*
2547+
createNewConnection(constchar*name)
25432548
{
25442549
remoteConnHashEnt*hentry;
25452550
boolfound;
@@ -2553,18 +2558,15 @@ createNewConnection(const char *name, remoteConn *rconn)
25532558
hentry= (remoteConnHashEnt*)hash_search(remoteConnHash,key,
25542559
HASH_ENTER,&found);
25552560

2556-
if (found)
2557-
{
2558-
libpqsrv_disconnect(rconn->conn);
2559-
pfree(rconn);
2560-
2561+
if (found&&hentry->rconn.conn!=NULL)
25612562
ereport(ERROR,
25622563
(errcode(ERRCODE_DUPLICATE_OBJECT),
25632564
errmsg("duplicate connection name")));
2564-
}
25652565

2566-
hentry->rconn=rconn;
2567-
strlcpy(hentry->name,name,sizeof(hentry->name));
2566+
/* New, or reusable, so initialize the rconn struct to zeroes */
2567+
memset(&hentry->rconn,0,sizeof(remoteConn));
2568+
2569+
return&hentry->rconn;
25682570
}
25692571

25702572
staticvoid
@@ -2592,9 +2594,12 @@ deleteConnection(const char *name)
25922594
* We need to make sure that the connection made used credentials
25932595
* which were provided by the user, so check what credentials were
25942596
* used to connect and then make sure that they came from the user.
2597+
*
2598+
* On failure, we close "conn" and also delete the hashtable entry
2599+
* identified by "connname" (if that's not NULL).
25952600
*/
25962601
staticvoid
2597-
dblink_security_check(PGconn*conn,remoteConn*rconn,constchar*connstr)
2602+
dblink_security_check(PGconn*conn,constchar*connname,constchar*connstr)
25982603
{
25992604
/* Superuser bypasses security check */
26002605
if (superuser())
@@ -2612,8 +2617,8 @@ dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr)
26122617

26132618
/* Otherwise, fail out */
26142619
libpqsrv_disconnect(conn);
2615-
if (rconn)
2616-
pfree(rconn);
2620+
if (connname)
2621+
deleteConnection(connname);
26172622

26182623
ereport(ERROR,
26192624
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),

0 commit comments

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