From f134421b1cf7c281b5429935981cae91e0ab4d78 Mon Sep 17 00:00:00 2001 From: Muzzammil Sarwar Date: Sun, 11 Oct 2026 17:58:31 +0500 Subject: [PATCH v1] Fix leak when a plpgsql exception block catches an error from CALL. When a non-atomic PL/pgSQL procedure, or any DO block, executes a CALL or DO statement, it pins the statement's plan in a resource owner that lives as long as the procedure or DO block, so that the pin survives a COMMIT or ROLLBACK in the called procedure. SPI releases the pin when the statement completes, but if the CALL fails, that step is skipped. If an exception block then catches the error, nothing releases the pin until the procedure or DO block exits, because rolling back the exception block's subtransaction does not touch that resource owner. A CALL whose arguments use PL/pgSQL variables is given a new custom plan each time it is executed, so each such failure kept a CachedPlan of about 2kB, and a procedure that catches failing CALLs in a loop could use memory without limit. DO blocks were affected in atomic contexts as well, since plpgsql_inline_handler() passes the block's simple-expression resource owner as the procedure resource owner whether or not the block is atomic. To fix, use the procedure-lifespan resource owner only when the called procedure could end the transaction, that is, when SPI will run the CALL non-atomically: in a non-atomic context with no subtransaction active, as SPI_inside_nonatomic_context() reports. Otherwise let SPI use the current resource owner, as it does for other statements. An error from the CALL can only be caught inside a subtransaction, and aborting that subtransaction now releases the pin. Oversight in commit ee895a655, so back-patch to v14. --- src/pl/plpgsql/src/expected/plpgsql_call.out | 64 ++++++++++++++++++ src/pl/plpgsql/src/pl_exec.c | 15 +++-- src/pl/plpgsql/src/sql/plpgsql_call.sql | 69 ++++++++++++++++++++ 3 files changed, 144 insertions(+), 4 deletions(-) diff --git a/src/pl/plpgsql/src/expected/plpgsql_call.out b/src/pl/plpgsql/src/expected/plpgsql_call.out index 3d0b117f236..da85ea07c77 100644 --- a/src/pl/plpgsql/src/expected/plpgsql_call.out +++ b/src/pl/plpgsql/src/expected/plpgsql_call.out @@ -635,3 +635,67 @@ NOTICE: f_print_x(1) NOTICE: f_get_x(2) NOTICE: f_print_x(2) ROLLBACK; +-- A CALL whose error is caught by an exception block must not leave its +-- plan pinned until the calling procedure or DO block exits. Count the +-- session's cached plans before and after ten caught failures, in a second +-- round, so that the first round has created every plan the loop needs. +CREATE PROCEDURE test_proc_error(x int) +LANGUAGE plpgsql +AS $$ +BEGIN + RAISE EXCEPTION 'error %', x; +END; +$$; +CREATE PROCEDURE test_proc_catch() +LANGUAGE plpgsql +AS $$ +DECLARE + before_count int; + after_count int; +BEGIN + FOR round IN 1..2 LOOP + before_count := (SELECT count(*) FROM pg_backend_memory_contexts + WHERE name = 'CachedPlan'); + FOR i IN 1..10 LOOP + BEGIN + CALL test_proc_error(i); + EXCEPTION WHEN OTHERS THEN + NULL; + END; + END LOOP; + after_count := (SELECT count(*) FROM pg_backend_memory_contexts + WHERE name = 'CachedPlan'); + END LOOP; + RAISE NOTICE 'cached plans kept: %', after_count - before_count; +END; +$$; +CALL test_proc_catch(); +NOTICE: cached plans kept: 0 +-- Likewise in a DO block, which has a long-lived resowner of its own even +-- in an atomic context +BEGIN; +DO $$ +DECLARE + before_count int; + after_count int; +BEGIN + FOR round IN 1..2 LOOP + before_count := (SELECT count(*) FROM pg_backend_memory_contexts + WHERE name = 'CachedPlan'); + FOR i IN 1..10 LOOP + BEGIN + CALL test_proc_error(i); + EXCEPTION WHEN OTHERS THEN + NULL; + END; + END LOOP; + after_count := (SELECT count(*) FROM pg_backend_memory_contexts + WHERE name = 'CachedPlan'); + END LOOP; + RAISE NOTICE 'cached plans kept: %', after_count - before_count; +END +$$; +NOTICE: cached plans kept: 0 +COMMIT; +DROP PROCEDURE test_proc_catch; +DROP PROCEDURE test_proc_error; diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index bc1667f7a13..e375f8e0432 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -2258,9 +2258,15 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt) before_lxid = MyProc->vxid.lxid; /* - * If we have a procedure-lifespan resowner, use that to hold the refcount - * for the plan. This avoids refcount leakage complaints if the called - * procedure ends the current transaction. + * If the called procedure could end the current transaction, hold the + * refcount for the plan in the procedure-lifespan resowner, which avoids + * refcount leakage complaints if it does. That can happen only when SPI + * executes the CALL non-atomically, which requires a non-atomic context + * and no active subtransaction (see _SPI_execute_plan). In other cases + * let SPI use the current resowner. That matters when the CALL fails + * with an error that an exception block catches: aborting the block's + * subtransaction then releases the refcount, which the procedure-lifespan + * resowner would otherwise keep until we exit. * * Also, tell SPI to allow non-atomic execution. */ @@ -2268,7 +2274,8 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt) options.params = paramLI; options.read_only = estate->readonly_func; options.allow_nonatomic = true; - options.owner = estate->procedure_resowner; + if (SPI_inside_nonatomic_context()) + options.owner = estate->procedure_resowner; rc = SPI_execute_plan_extended(expr->plan, &options); diff --git a/src/pl/plpgsql/src/sql/plpgsql_call.sql b/src/pl/plpgsql/src/sql/plpgsql_call.sql index 08c1659ef15..06a8914ec58 100644 --- a/src/pl/plpgsql/src/sql/plpgsql_call.sql +++ b/src/pl/plpgsql/src/sql/plpgsql_call.sql @@ -589,3 +589,72 @@ END $$; ROLLBACK; + +-- A CALL whose error is caught by an exception block must not leave its +-- plan pinned until the calling procedure or DO block exits. Count the +-- session's cached plans before and after ten caught failures, in a second +-- round, so that the first round has created every plan the loop needs. +CREATE PROCEDURE test_proc_error(x int) +LANGUAGE plpgsql +AS $$ +BEGIN + RAISE EXCEPTION 'error %', x; +END; +$$; + +CREATE PROCEDURE test_proc_catch() +LANGUAGE plpgsql +AS $$ +DECLARE + before_count int; + after_count int; +BEGIN + FOR round IN 1..2 LOOP + before_count := (SELECT count(*) FROM pg_backend_memory_contexts + WHERE name = 'CachedPlan'); + FOR i IN 1..10 LOOP + BEGIN + CALL test_proc_error(i); + EXCEPTION WHEN OTHERS THEN + NULL; + END; + END LOOP; + after_count := (SELECT count(*) FROM pg_backend_memory_contexts + WHERE name = 'CachedPlan'); + END LOOP; + RAISE NOTICE 'cached plans kept: %', after_count - before_count; +END; +$$; + +CALL test_proc_catch(); + +-- Likewise in a DO block, which has a long-lived resowner of its own even +-- in an atomic context +BEGIN; + +DO $$ +DECLARE + before_count int; + after_count int; +BEGIN + FOR round IN 1..2 LOOP + before_count := (SELECT count(*) FROM pg_backend_memory_contexts + WHERE name = 'CachedPlan'); + FOR i IN 1..10 LOOP + BEGIN + CALL test_proc_error(i); + EXCEPTION WHEN OTHERS THEN + NULL; + END; + END LOOP; + after_count := (SELECT count(*) FROM pg_backend_memory_contexts + WHERE name = 'CachedPlan'); + END LOOP; + RAISE NOTICE 'cached plans kept: %', after_count - before_count; +END +$$; + +COMMIT; + +DROP PROCEDURE test_proc_catch; +DROP PROCEDURE test_proc_error; -- 2.47.3