Skip to content

Commit 5e4a0b7

Browse files
tglsfdctglsfdc
committed
Avoid holding a directory FD open across assorted SRF calls.
This extends the fixes made in commit 085b6b6 to other SRFs with the same bug, namely pg_logdir_ls(), pgrowlocks(), pg_timezone_names(), pg_ls_dir(), and pg_tablespace_databases(). Also adjust various comments and documentation to warn against expecting to clean up resources during a ValuePerCall SRF's final call. Back-patch to all supported branches, since these functions were all born broken. Justin Pryzby, with cosmetic tweaks by me Discussion: https://postgr.es/m/20200308173103.GC1357@telsasoft.com
1 parent c6b75b3 commit 5e4a0b7

File tree

10 files changed

+391
-347
lines changed

10 files changed

+391
-347
lines changed

‎contrib/adminpack/adminpack.c

Lines changed: 38 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,6 @@ PG_FUNCTION_INFO_V1(pg_file_rename);
4545
PG_FUNCTION_INFO_V1(pg_file_unlink);
4646
PG_FUNCTION_INFO_V1(pg_logdir_ls);
4747

48-
typedef struct
49-
{
50-
char *location;
51-
DIR *dirdesc;
52-
} directory_fctx;
5348

5449
/*-----------------------
5550
* some helper functions
@@ -281,9 +276,14 @@ pg_file_unlink(PG_FUNCTION_ARGS)
281276
Datum
282277
pg_logdir_ls(PG_FUNCTION_ARGS)
283278
{
284-
FuncCallContext *funcctx;
279+
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
280+
bool randomAccess;
281+
TupleDesc tupdesc;
282+
Tuplestorestate *tupstore;
283+
AttInMetadata *attinmeta;
284+
DIR *dirdesc;
285285
struct dirent *de;
286-
directory_fctx *fctx;
286+
MemoryContext oldcontext;
287287

288288
if (!superuser())
289289
ereport(ERROR,
@@ -293,43 +293,39 @@ pg_logdir_ls(PG_FUNCTION_ARGS)
293293
if (strcmp(Log_filename, "postgresql-%Y-%m-%d_%H%M%S.log") != 0)
294294
ereport(ERROR,
295295
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
296-
(errmsg("the log_filename parameter must equal 'postgresql-%%Y-%%m-%%d_%%H%%M%%S.log'"))));
297-
298-
if (SRF_IS_FIRSTCALL())
299-
{
300-
MemoryContext oldcontext;
301-
TupleDesc tupdesc;
302-
303-
funcctx = SRF_FIRSTCALL_INIT();
304-
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
296+
errmsg("the log_filename parameter must equal 'postgresql-%%Y-%%m-%%d_%%H%%M%%S.log'")));
305297

306-
fctx = palloc(sizeof(directory_fctx));
307-
308-
tupdesc = CreateTemplateTupleDesc(2, false);
309-
TupleDescInitEntry(tupdesc, (AttrNumber) 1, "starttime",
310-
TIMESTAMPOID, -1, 0);
311-
TupleDescInitEntry(tupdesc, (AttrNumber) 2, "filename",
312-
TEXTOID, -1, 0);
298+
/* check to see if caller supports us returning a tuplestore */
299+
if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
300+
ereport(ERROR,
301+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
302+
errmsg("set-valued function called in context that cannot accept a set")));
303+
if (!(rsinfo->allowedModes & SFRM_Materialize))
304+
ereport(ERROR,
305+
(errcode(ERRCODE_SYNTAX_ERROR),
306+
errmsg("materialize mode required, but it is not allowed in this context")));
313307

314-
funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);
308+
/* The tupdesc and tuplestore must be created in ecxt_per_query_memory */
309+
oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
315310

316-
fctx->location = pstrdup(Log_directory);
317-
fctx->dirdesc = AllocateDir(fctx->location);
311+
tupdesc = CreateTemplateTupleDesc(2, false);
312+
TupleDescInitEntry(tupdesc, (AttrNumber) 1, "starttime",
313+
TIMESTAMPOID, -1, 0);
314+
TupleDescInitEntry(tupdesc, (AttrNumber) 2, "filename",
315+
TEXTOID, -1, 0);
318316

319-
if (!fctx->dirdesc)
320-
ereport(ERROR,
321-
(errcode_for_file_access(),
322-
errmsg("could not read directory \"%s\": %m",
323-
fctx->location)));
317+
randomAccess = (rsinfo->allowedModes & SFRM_Materialize_Random) != 0;
318+
tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
319+
rsinfo->returnMode = SFRM_Materialize;
320+
rsinfo->setResult = tupstore;
321+
rsinfo->setDesc = tupdesc;
324322

325-
funcctx->user_fctx = fctx;
326-
MemoryContextSwitchTo(oldcontext);
327-
}
323+
MemoryContextSwitchTo(oldcontext);
328324

329-
funcctx = SRF_PERCALL_SETUP();
330-
fctx = (directory_fctx *) funcctx->user_fctx;
325+
attinmeta = TupleDescGetAttInMetadata(tupdesc);
331326

332-
while ((de = ReadDir(fctx->dirdesc, fctx->location)) != NULL)
327+
dirdesc = AllocateDir(Log_directory);
328+
while ((de = ReadDir(dirdesc, Log_directory)) != NULL)
333329
{
334330
char *values[2];
335331
HeapTuple tuple;
@@ -366,13 +362,13 @@ pg_logdir_ls(PG_FUNCTION_ARGS)
366362
/* Seems the timestamp is OK; prepare and return tuple */
367363

368364
values[0] = timestampbuf;
369-
values[1] = psprintf("%s/%s", fctx->location, de->d_name);
365+
values[1] = psprintf("%s/%s", Log_directory, de->d_name);
370366

371-
tuple = BuildTupleFromCStrings(funcctx->attinmeta, values);
367+
tuple = BuildTupleFromCStrings(attinmeta, values);
372368

373-
SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(tuple));
369+
tuplestore_puttuple(tupstore, tuple);
374370
}
375371

376-
FreeDir(fctx->dirdesc);
377-
SRF_RETURN_DONE(funcctx);
372+
FreeDir(dirdesc);
373+
return (Datum) 0;
378374
}

‎contrib/pgrowlocks/pgrowlocks.c

Lines changed: 73 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,6 @@ PG_FUNCTION_INFO_V1(pgrowlocks);
5252

5353
#define NCHARS 32
5454

55-
typedef struct
56-
{
57-
Relation rel;
58-
HeapScanDesc scan;
59-
int ncolumns;
60-
} MyData;
61-
6255
#define Atnum_tid 0
6356
#define Atnum_xmax 1
6457
#define Atnum_ismulti 2
@@ -69,77 +62,80 @@ typedef struct
6962
Datum
7063
pgrowlocks(PG_FUNCTION_ARGS)
7164
{
72-
FuncCallContext *funcctx;
73-
HeapScanDesc scan;
74-
HeapTuple tuple;
65+
text *relname = PG_GETARG_TEXT_PP(0);
66+
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
67+
bool randomAccess;
7568
TupleDesc tupdesc;
69+
Tuplestorestate *tupstore;
7670
AttInMetadata *attinmeta;
77-
Datum result;
78-
MyData *mydata;
7971
Relation rel;
72+
RangeVar *relrv;
73+
HeapScanDesc scan;
74+
HeapTuple tuple;
75+
MemoryContext oldcontext;
76+
AclResult aclresult;
77+
char **values;
78+
79+
/* check to see if caller supports us returning a tuplestore */
80+
if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
81+
ereport(ERROR,
82+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
83+
errmsg("set-valued function called in context that cannot accept a set")));
84+
if (!(rsinfo->allowedModes & SFRM_Materialize))
85+
ereport(ERROR,
86+
(errcode(ERRCODE_SYNTAX_ERROR),
87+
errmsg("materialize mode required, but it is not allowed in this context")));
88+
89+
/* The tupdesc and tuplestore must be created in ecxt_per_query_memory */
90+
oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
91+
92+
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
93+
elog(ERROR, "return type must be a row type");
94+
95+
randomAccess = (rsinfo->allowedModes & SFRM_Materialize_Random) != 0;
96+
tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
97+
rsinfo->returnMode = SFRM_Materialize;
98+
rsinfo->setResult = tupstore;
99+
rsinfo->setDesc = tupdesc;
100+
101+
MemoryContextSwitchTo(oldcontext);
102+
103+
/* Access the table */
104+
relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
105+
rel = relation_openrv(relrv, AccessShareLock);
106+
107+
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
108+
ereport(ERROR,
109+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
110+
errmsg("\"%s\" is a partitioned table",
111+
RelationGetRelationName(rel)),
112+
errdetail("Partitioned tables do not contain rows.")));
113+
else if (rel->rd_rel->relkind != RELKIND_RELATION)
114+
ereport(ERROR,
115+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
116+
errmsg("\"%s\" is not a table",
117+
RelationGetRelationName(rel))));
118+
119+
/*
120+
* check permissions: must have SELECT on table or be in
121+
* pg_stat_scan_tables
122+
*/
123+
aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
124+
ACL_SELECT);
125+
if (aclresult != ACLCHECK_OK)
126+
aclresult = is_member_of_role(GetUserId(), DEFAULT_ROLE_STAT_SCAN_TABLES) ? ACLCHECK_OK : ACLCHECK_NO_PRIV;
127+
128+
if (aclresult != ACLCHECK_OK)
129+
aclcheck_error(aclresult, ACL_KIND_CLASS,
130+
RelationGetRelationName(rel));
131+
132+
/* Scan the relation */
133+
scan = heap_beginscan(rel, GetActiveSnapshot(), 0, NULL);
134+
135+
attinmeta = TupleDescGetAttInMetadata(tupdesc);
136+
137+
values = (char **) palloc(tupdesc->natts * sizeof(char *));
80138

81-
if (SRF_IS_FIRSTCALL())
82-
{
83-
text *relname;
84-
RangeVar *relrv;
85-
MemoryContext oldcontext;
86-
AclResult aclresult;
87-
88-
funcctx = SRF_FIRSTCALL_INIT();
89-
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
90-
91-
/* Build a tuple descriptor for our result type */
92-
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
93-
elog(ERROR, "return type must be a row type");
94-
95-
attinmeta = TupleDescGetAttInMetadata(tupdesc);
96-
funcctx->attinmeta = attinmeta;
97-
98-
relname = PG_GETARG_TEXT_PP(0);
99-
relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
100-
rel = relation_openrv(relrv, AccessShareLock);
101-
102-
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
103-
ereport(ERROR,
104-
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
105-
errmsg("\"%s\" is a partitioned table",
106-
RelationGetRelationName(rel)),
107-
errdetail("Partitioned tables do not contain rows.")));
108-
else if (rel->rd_rel->relkind != RELKIND_RELATION)
109-
ereport(ERROR,
110-
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
111-
errmsg("\"%s\" is not a table",
112-
RelationGetRelationName(rel))));
113-
114-
/*
115-
* check permissions: must have SELECT on table or be in
116-
* pg_stat_scan_tables
117-
*/
118-
aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
119-
ACL_SELECT);
120-
if (aclresult != ACLCHECK_OK)
121-
aclresult = is_member_of_role(GetUserId(), DEFAULT_ROLE_STAT_SCAN_TABLES) ? ACLCHECK_OK : ACLCHECK_NO_PRIV;
122-
123-
if (aclresult != ACLCHECK_OK)
124-
aclcheck_error(aclresult, ACL_KIND_CLASS,
125-
RelationGetRelationName(rel));
126-
127-
scan = heap_beginscan(rel, GetActiveSnapshot(), 0, NULL);
128-
mydata = palloc(sizeof(*mydata));
129-
mydata->rel = rel;
130-
mydata->scan = scan;
131-
mydata->ncolumns = tupdesc->natts;
132-
funcctx->user_fctx = mydata;
133-
134-
MemoryContextSwitchTo(oldcontext);
135-
}
136-
137-
funcctx = SRF_PERCALL_SETUP();
138-
attinmeta = funcctx->attinmeta;
139-
mydata = (MyData *) funcctx->user_fctx;
140-
scan = mydata->scan;
141-
142-
/* scan the relation */
143139
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
144140
{
145141
HTSU_Result htsu;
@@ -160,10 +156,6 @@ pgrowlocks(PG_FUNCTION_ARGS)
160156
*/
161157
if (htsu == HeapTupleBeingUpdated)
162158
{
163-
char **values;
164-
165-
values = (char **) palloc(mydata->ncolumns * sizeof(char *));
166-
167159
values[Atnum_tid] = (char *) DirectFunctionCall1(tidout,
168160
PointerGetDatum(&tuple->t_self));
169161

@@ -288,16 +280,7 @@ pgrowlocks(PG_FUNCTION_ARGS)
288280

289281
/* build a tuple */
290282
tuple = BuildTupleFromCStrings(attinmeta, values);
291-
292-
/* make the tuple into a datum */
293-
result = HeapTupleGetDatum(tuple);
294-
295-
/*
296-
* no need to pfree what we allocated; it's on a short-lived
297-
* memory context anyway
298-
*/
299-
300-
SRF_RETURN_NEXT(funcctx, result);
283+
tuplestore_puttuple(tupstore, tuple);
301284
}
302285
else
303286
{
@@ -306,7 +289,6 @@ pgrowlocks(PG_FUNCTION_ARGS)
306289
}
307290

308291
heap_endscan(scan);
309-
heap_close(mydata->rel, AccessShareLock);
310-
311-
SRF_RETURN_DONE(funcctx);
292+
heap_close(rel, AccessShareLock);
293+
return (Datum) 0;
312294
}

0 commit comments

Comments
 (0)