Skip to content

Commit 9603214

Browse files
committed
PLpgSQL_estate estate is from stack, and it can be corrupted immediately after leaving plpgsql_exec_function.
When I prepare calling of func_abort, I cannot to use any field from estate, and I have to use local estate instead.
1 parent af1e9a9 commit 9603214

3 files changed

Lines changed: 59 additions & 11 deletions

File tree

src/cursors_leaks.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ int plpgsql_check_cursors_leaks_level = WARNING;
3333
typedef struct CursorTrace
3434
{
3535
int stmtid;
36-
PLpgSQL_execstate *estate;
36+
void *plugin_info;
3737
char *curname;
3838
} CursorTrace;
3939

@@ -195,7 +195,7 @@ func_end(PLpgSQL_execstate *estate,
195195
* Iterate over traced cursors. Remove slots for tracing immediately,
196196
* when traced cursor is closed already.
197197
*/
198-
if (ct->curname && ct->estate == estate)
198+
if (ct->curname && ct->plugin_info == estate->plugin_info)
199199
{
200200
if (SPI_cursor_find(ct->curname))
201201
{
@@ -344,7 +344,7 @@ stmt_end(PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt, plch_fextra *fextra)
344344

345345
ct->stmtid = stmt->stmtid;
346346

347-
ct->estate = estate;
347+
ct->plugin_info = estate->plugin_info;
348348
ct->curname = pstrdup(curname);
349349

350350
MemoryContextSwitchTo(oldcxt);

src/fextra_cache.c

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ fextra_init_hk(plch_fextra_hk *hk, PLpgSQL_function *func)
187187

188188
}
189189

190+
190191
static void
191192
fextra_CacheObjectCallback(Datum arg, int cacheid, uint32 hashValue)
192193
{
@@ -206,6 +207,17 @@ fextra_CacheObjectCallback(Datum arg, int cacheid, uint32 hashValue)
206207

207208
if (!fextra->is_valid && fextra->use_count == 0)
208209
{
210+
211+
#if PG_VERSION_NUM >= 180000
212+
213+
fextra->func->cfunc.use_count--;
214+
215+
#else
216+
217+
fextra->func->use_count--;
218+
219+
#endif
220+
209221
MemoryContextDelete(fextra->mcxt);
210222
if (hash_search(fextra_ht,
211223
&fextra->hk,
@@ -257,6 +269,17 @@ plch_get_fextra(PLpgSQL_function *func)
257269

258270
if (found && !fextra->is_valid && fextra->use_count == 0)
259271
{
272+
273+
#if PG_VERSION_NUM >= 180000
274+
275+
fextra->func->cfunc.use_count--;
276+
277+
#else
278+
279+
fextra->func->use_count--;
280+
281+
#endif
282+
260283
MemoryContextReset(fextra->mcxt);
261284
}
262285

@@ -317,22 +340,23 @@ plch_get_fextra(PLpgSQL_function *func)
317340
fextra->max_deep = 0;
318341
init_fextra_stmt(fextra, 0, &naturalid, 1, 0, (PLpgSQL_stmt *) func->action);
319342

320-
fextra->is_valid = true;
321-
}
322-
323-
fextra->use_count++;
324-
fextra->func = func;
325-
326343
#if PG_VERSION_NUM >= 180000
327344

328-
func->cfunc.use_count++;
345+
func->cfunc.use_count++;
329346

330347
#else
331348

332-
func->use_count++;
349+
func->use_count++;
333350

334351
#endif
335352

353+
fextra->func = func;
354+
355+
fextra->is_valid = true;
356+
}
357+
358+
fextra->use_count++;
359+
336360
return fextra;
337361
}
338362

src/pldbgapi3.c

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,8 @@ static void
158158
plugin_info_reset(void *arg)
159159
{
160160
plpgsql_plugin_info *plugin_info = (plpgsql_plugin_info*) arg;
161+
PLpgSQL_execstate loc_estate;
162+
PLpgSQL_execstate *old_cur_estate;
161163
MemoryContext exec_mcxt = CurrentMemoryContext;
162164
int stmts_stack_size;
163165
int i;
@@ -173,6 +175,19 @@ plugin_info_reset(void *arg)
173175
if (!plugin_info->fextra)
174176
return;
175177

178+
/*
179+
* In this moment, the estate content can be corrupted, because
180+
* estate is from stack of already ended function.
181+
*
182+
* Inside abort methods, the estate fields should not be referenced!
183+
*/
184+
memset(&loc_estate, 0, sizeof(PLpgSQL_execstate));
185+
loc_estate.func = plugin_info->fextra->func;
186+
187+
old_cur_estate = loc_estate.func->cur_estate;
188+
loc_estate.func->cur_estate = &loc_estate;
189+
plugin_info->estate = &loc_estate;
190+
176191
stmts_stack_size = plugin_info->stmts_stack_size;
177192
plugin_info->stmts_stack_size = 0;
178193

@@ -189,6 +204,11 @@ plugin_info_reset(void *arg)
189204
plugin_info->estate->plugin_info = plugin_info->plugin_info[i];
190205

191206
MemoryContextSwitchTo(exec_mcxt);
207+
208+
/*
209+
* In this moment, the estate content can be corrupted.
210+
* Inside abort methods, the estate fields should not be referenced!
211+
*/
192212
plugins[i]->func_abort(plugin_info->estate,
193213
plugin_info->estate->func,
194214
plugin_info->fextra);
@@ -198,15 +218,19 @@ plugin_info_reset(void *arg)
198218

199219
PG_CATCH();
200220
{
221+
plugin_info->fextra->func->cur_estate = old_cur_estate;
201222
plch_release_fextra(plugin_info->fextra);
202223
plugin_info->fextra = NULL;
224+
plugin_info->estate = NULL;
203225

204226
PG_RE_THROW();
205227
}
206228
PG_END_TRY();
207229

230+
plugin_info->fextra->func->cur_estate = old_cur_estate;
208231
plch_release_fextra(plugin_info->fextra);
209232
plugin_info->fextra = NULL;
233+
plugin_info->estate = NULL;
210234
}
211235

212236
/*

0 commit comments

Comments
 (0)