Skip to content

Commit 04eb75e

Browse files
hlinnakahlinnaka
committed
pageinspect: Fix relcache leak in gist_page_items().
The gist_page_items() function opened the index relation on first call and closed it on the last call. But there's no guarantee that the function is run to completion, leading to a relcache leak and warning at the end of the transaction. To fix, refactor the function to return all the rows in one call, as a tuplestore. Reported-by: Tom Lane Discussion: https://www.postgresql.org/message-id/234863.1610916631%40sss.pgh.pa.us
1 parent 7db0cd2 commit 04eb75e

File tree

1 file changed

+74
-96
lines changed

1 file changed

+74
-96
lines changed

‎contrib/pageinspect/gistfuncs.c

+74-96
Original file line numberDiff line numberDiff line change
@@ -92,65 +92,56 @@ gist_page_opaque_info(PG_FUNCTION_ARGS)
9292
return HeapTupleGetDatum(resultTuple);
9393
}
9494

95-
typedef struct gist_page_items_state
96-
{
97-
Page page;
98-
TupleDesc tupd;
99-
OffsetNumber offset;
100-
Relation rel;
101-
} gist_page_items_state;
102-
10395
Datum
10496
gist_page_items_bytea(PG_FUNCTION_ARGS)
10597
{
10698
bytea *raw_page = PG_GETARG_BYTEA_P(0);
107-
FuncCallContext *fctx;
108-
gist_page_items_state *inter_call_data;
99+
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
100+
bool randomAccess;
101+
TupleDesc tupdesc;
102+
Tuplestorestate *tupstore;
103+
MemoryContext oldcontext;
104+
Page page;
105+
OffsetNumber offset;
109106

110107
if (!superuser())
111108
ereport(ERROR,
112109
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
113110
errmsg("must be superuser to use raw page functions")));
114111

115-
if (SRF_IS_FIRSTCALL())
116-
{
117-
TupleDesc tupdesc;
118-
MemoryContext mctx;
119-
Page page;
120-
121-
fctx = SRF_FIRSTCALL_INIT();
122-
mctx = MemoryContextSwitchTo(fctx->multi_call_memory_ctx);
123-
124-
page = get_page_from_raw(raw_page);
125-
126-
inter_call_data = palloc(sizeof(gist_page_items_state));
112+
/* check to see if caller supports us returning a tuplestore */
113+
if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
114+
ereport(ERROR,
115+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
116+
errmsg("set-valued function called in context that cannot accept a set")));
117+
if (!(rsinfo->allowedModes & SFRM_Materialize))
118+
ereport(ERROR,
119+
(errcode(ERRCODE_SYNTAX_ERROR),
120+
errmsg("materialize mode required, but it is not allowed in this context")));
127121

128-
/* Build a tuple descriptor for our result type */
129-
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
130-
elog(ERROR, "return type must be a row type");
122+
/* The tupdesc and tuplestore must be created in ecxt_per_query_memory */
123+
oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
131124

132-
if (GistPageIsDeleted(page))
133-
elog(NOTICE, "page is deleted");
125+
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
126+
elog(ERROR, "return type must be a row type");
134127

135-
inter_call_data->page = page;
136-
inter_call_data->tupd = tupdesc;
137-
inter_call_data->offset = FirstOffsetNumber;
128+
randomAccess = (rsinfo->allowedModes & SFRM_Materialize_Random) != 0;
129+
tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
130+
rsinfo->returnMode = SFRM_Materialize;
131+
rsinfo->setResult = tupstore;
132+
rsinfo->setDesc = tupdesc;
138133

139-
fctx->max_calls = PageGetMaxOffsetNumber(page);
140-
fctx->user_fctx = inter_call_data;
134+
MemoryContextSwitchTo(oldcontext);
141135

142-
MemoryContextSwitchTo(mctx);
143-
}
136+
page = get_page_from_raw(raw_page);
144137

145-
fctx = SRF_PERCALL_SETUP();
146-
inter_call_data = fctx->user_fctx;
138+
if (GistPageIsDeleted(page))
139+
elog(NOTICE, "page is deleted");
147140

148-
if (fctx->call_cntr < fctx->max_calls)
141+
for (offset = FirstOffsetNumber;
142+
offset <= PageGetMaxOffsetNumber(page);
143+
offset++)
149144
{
150-
Page page = inter_call_data->page;
151-
OffsetNumber offset = inter_call_data->offset;
152-
HeapTuple resultTuple;
153-
Datum result;
154145
Datum values[4];
155146
bool nulls[4];
156147
ItemId id;
@@ -177,74 +168,67 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
177168
memcpy(VARDATA(tuple_bytea), itup, tuple_len);
178169
values[3] = PointerGetDatum(tuple_bytea);
179170

180-
/* Build and return the result tuple. */
181-
resultTuple = heap_form_tuple(inter_call_data->tupd, values, nulls);
182-
result = HeapTupleGetDatum(resultTuple);
183-
184-
inter_call_data->offset++;
185-
SRF_RETURN_NEXT(fctx, result);
171+
tuplestore_putvalues(tupstore, tupdesc, values, nulls);
186172
}
187173

188-
SRF_RETURN_DONE(fctx);
174+
return (Datum) 0;
189175
}
190176

191177
Datum
192178
gist_page_items(PG_FUNCTION_ARGS)
193179
{
194180
bytea *raw_page = PG_GETARG_BYTEA_P(0);
195181
Oid indexRelid = PG_GETARG_OID(1);
196-
FuncCallContext *fctx;
197-
gist_page_items_state *inter_call_data;
182+
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
183+
bool randomAccess;
184+
Relation indexRel;
185+
TupleDesc tupdesc;
186+
Tuplestorestate *tupstore;
187+
MemoryContext oldcontext;
188+
Page page;
189+
OffsetNumber offset;
198190

199191
if (!superuser())
200192
ereport(ERROR,
201193
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
202194
errmsg("must be superuser to use raw page functions")));
203195

204-
if (SRF_IS_FIRSTCALL())
205-
{
206-
Relation indexRel;
207-
TupleDesc tupdesc;
208-
MemoryContext mctx;
209-
Page page;
210-
211-
fctx = SRF_FIRSTCALL_INIT();
212-
mctx = MemoryContextSwitchTo(fctx->multi_call_memory_ctx);
213-
214-
page = get_page_from_raw(raw_page);
215-
216-
inter_call_data = palloc(sizeof(gist_page_items_state));
196+
/* check to see if caller supports us returning a tuplestore */
197+
if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
198+
ereport(ERROR,
199+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
200+
errmsg("set-valued function called in context that cannot accept a set")));
201+
if (!(rsinfo->allowedModes & SFRM_Materialize))
202+
ereport(ERROR,
203+
(errcode(ERRCODE_SYNTAX_ERROR),
204+
errmsg("materialize mode required, but it is not allowed in this context")));
217205

218-
/* Open the relation */
219-
indexRel = index_open(indexRelid, AccessShareLock);
206+
/* The tupdesc and tuplestore must be created in ecxt_per_query_memory */
207+
oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
220208

221-
/* Build a tuple descriptor for our result type */
222-
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
223-
elog(ERROR, "return type must be a row type");
209+
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
210+
elog(ERROR, "return type must be a row type");
224211

225-
if (GistPageIsDeleted(page))
226-
elog(NOTICE, "page is deleted");
212+
randomAccess = (rsinfo->allowedModes & SFRM_Materialize_Random) != 0;
213+
tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
214+
rsinfo->returnMode = SFRM_Materialize;
215+
rsinfo->setResult = tupstore;
216+
rsinfo->setDesc = tupdesc;
227217

228-
inter_call_data->page = page;
229-
inter_call_data->tupd = tupdesc;
230-
inter_call_data->offset = FirstOffsetNumber;
231-
inter_call_data->rel = indexRel;
218+
MemoryContextSwitchTo(oldcontext);
232219

233-
fctx->max_calls = PageGetMaxOffsetNumber(page);
234-
fctx->user_fctx = inter_call_data;
220+
/* Open the relation */
221+
indexRel = index_open(indexRelid, AccessShareLock);
235222

236-
MemoryContextSwitchTo(mctx);
237-
}
223+
page = get_page_from_raw(raw_page);
238224

239-
fctx = SRF_PERCALL_SETUP();
240-
inter_call_data = fctx->user_fctx;
225+
if (GistPageIsDeleted(page))
226+
elog(NOTICE, "page is deleted");
241227

242-
if (fctx->call_cntr < fctx->max_calls)
228+
for (offset = FirstOffsetNumber;
229+
offset <= PageGetMaxOffsetNumber(page);
230+
offset++)
243231
{
244-
Page page = inter_call_data->page;
245-
OffsetNumber offset = inter_call_data->offset;
246-
HeapTuple resultTuple;
247-
Datum result;
248232
Datum values[4];
249233
bool nulls[4];
250234
ItemId id;
@@ -260,11 +244,10 @@ gist_page_items(PG_FUNCTION_ARGS)
260244

261245
itup = (IndexTuple) PageGetItem(page, id);
262246

263-
index_deform_tuple(itup, RelationGetDescr(inter_call_data->rel),
247+
index_deform_tuple(itup, RelationGetDescr(indexRel),
264248
itup_values, itup_isnull);
265249

266-
key_desc = BuildIndexValueDescription(inter_call_data->rel, itup_values,
267-
itup_isnull);
250+
key_desc = BuildIndexValueDescription(indexRel, itup_values, itup_isnull);
268251

269252
memset(nulls, 0, sizeof(nulls));
270253

@@ -273,15 +256,10 @@ gist_page_items(PG_FUNCTION_ARGS)
273256
values[2] = Int32GetDatum((int) IndexTupleSize(itup));
274257
values[3] = CStringGetTextDatum(key_desc);
275258

276-
/* Build and return the result tuple. */
277-
resultTuple = heap_form_tuple(inter_call_data->tupd, values, nulls);
278-
result = HeapTupleGetDatum(resultTuple);
279-
280-
inter_call_data->offset++;
281-
SRF_RETURN_NEXT(fctx, result);
259+
tuplestore_putvalues(tupstore, tupdesc, values, nulls);
282260
}
283261

284-
relation_close(inter_call_data->rel, AccessShareLock);
262+
relation_close(indexRel, AccessShareLock);
285263

286-
SRF_RETURN_DONE(fctx);
264+
return (Datum) 0;
287265
}

0 commit comments

Comments
 (0)