pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Fix memory leakage in postgres_fdw's DirectModify code path. 6+ messages / 1 participants [nested] [flat]
* pgsql: Fix memory leakage in postgres_fdw's DirectModify code path. @ 2025-05-30 17:46 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2025-05-30 17:46 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix memory leakage in postgres_fdw's DirectModify code path. postgres_fdw tries to use PG_TRY blocks to ensure that it will eventually free the PGresult created by the remote modify command. However, it's fundamentally impossible for this scheme to work reliably when there's RETURNING data, because the query could fail in between invocations of postgres_fdw's DirectModify methods. There is at least one instance of exactly this situation in the regression tests, and the ensuing session-lifespan leak is visible under Valgrind. We can improve matters by using a memory context reset callback attached to the ExecutorState context. That ensures that the PGresult will be freed when the ExecutorState context is torn down, even if control never reaches postgresEndDirectModify. I have little faith that there aren't other potential PGresult leakages in the backend modules that use libpq. So I think it'd be a good idea to apply this concept universally by creating infrastructure that attaches a reset callback to every PGresult generated in the backend. However, that seems too invasive for v18 at this point, let alone the back branches. So for the moment, apply this narrow fix that just makes DirectModify safe. I have a patch in the queue for the more general idea, but it will have to wait for v19. Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com> Discussion: https://postgr.es/m/2976982.1748049023@sss.pgh.pa.us Backpatch-through: 13 Branch ------ REL_15_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/3c31594f55cd210bbee4e2ac92c2610b8c260f2c Modified Files -------------- contrib/postgres_fdw/postgres_fdw.c | 60 ++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 27 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix memory leakage in postgres_fdw's DirectModify code path. @ 2025-05-30 17:46 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2025-05-30 17:46 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix memory leakage in postgres_fdw's DirectModify code path. postgres_fdw tries to use PG_TRY blocks to ensure that it will eventually free the PGresult created by the remote modify command. However, it's fundamentally impossible for this scheme to work reliably when there's RETURNING data, because the query could fail in between invocations of postgres_fdw's DirectModify methods. There is at least one instance of exactly this situation in the regression tests, and the ensuing session-lifespan leak is visible under Valgrind. We can improve matters by using a memory context reset callback attached to the ExecutorState context. That ensures that the PGresult will be freed when the ExecutorState context is torn down, even if control never reaches postgresEndDirectModify. I have little faith that there aren't other potential PGresult leakages in the backend modules that use libpq. So I think it'd be a good idea to apply this concept universally by creating infrastructure that attaches a reset callback to every PGresult generated in the backend. However, that seems too invasive for v18 at this point, let alone the back branches. So for the moment, apply this narrow fix that just makes DirectModify safe. I have a patch in the queue for the more general idea, but it will have to wait for v19. Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com> Discussion: https://postgr.es/m/2976982.1748049023@sss.pgh.pa.us Backpatch-through: 13 Branch ------ REL_13_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/271cb7eaa7c8ff2d78487d2764748a193ae34fc0 Modified Files -------------- contrib/postgres_fdw/postgres_fdw.c | 60 ++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 27 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix memory leakage in postgres_fdw's DirectModify code path. @ 2025-05-30 17:46 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2025-05-30 17:46 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix memory leakage in postgres_fdw's DirectModify code path. postgres_fdw tries to use PG_TRY blocks to ensure that it will eventually free the PGresult created by the remote modify command. However, it's fundamentally impossible for this scheme to work reliably when there's RETURNING data, because the query could fail in between invocations of postgres_fdw's DirectModify methods. There is at least one instance of exactly this situation in the regression tests, and the ensuing session-lifespan leak is visible under Valgrind. We can improve matters by using a memory context reset callback attached to the ExecutorState context. That ensures that the PGresult will be freed when the ExecutorState context is torn down, even if control never reaches postgresEndDirectModify. I have little faith that there aren't other potential PGresult leakages in the backend modules that use libpq. So I think it'd be a good idea to apply this concept universally by creating infrastructure that attaches a reset callback to every PGresult generated in the backend. However, that seems too invasive for v18 at this point, let alone the back branches. So for the moment, apply this narrow fix that just makes DirectModify safe. I have a patch in the queue for the more general idea, but it will have to wait for v19. Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com> Discussion: https://postgr.es/m/2976982.1748049023@sss.pgh.pa.us Backpatch-through: 13 Branch ------ REL_14_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/4a07c096132c389a6d3e42e4af9078999a6a48e8 Modified Files -------------- contrib/postgres_fdw/postgres_fdw.c | 60 ++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 27 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix memory leakage in postgres_fdw's DirectModify code path. @ 2025-05-30 17:46 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2025-05-30 17:46 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix memory leakage in postgres_fdw's DirectModify code path. postgres_fdw tries to use PG_TRY blocks to ensure that it will eventually free the PGresult created by the remote modify command. However, it's fundamentally impossible for this scheme to work reliably when there's RETURNING data, because the query could fail in between invocations of postgres_fdw's DirectModify methods. There is at least one instance of exactly this situation in the regression tests, and the ensuing session-lifespan leak is visible under Valgrind. We can improve matters by using a memory context reset callback attached to the ExecutorState context. That ensures that the PGresult will be freed when the ExecutorState context is torn down, even if control never reaches postgresEndDirectModify. I have little faith that there aren't other potential PGresult leakages in the backend modules that use libpq. So I think it'd be a good idea to apply this concept universally by creating infrastructure that attaches a reset callback to every PGresult generated in the backend. However, that seems too invasive for v18 at this point, let alone the back branches. So for the moment, apply this narrow fix that just makes DirectModify safe. I have a patch in the queue for the more general idea, but it will have to wait for v19. Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com> Discussion: https://postgr.es/m/2976982.1748049023@sss.pgh.pa.us Backpatch-through: 13 Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/9339c85afc913752884858b28cca99b8f42a389c Modified Files -------------- contrib/postgres_fdw/postgres_fdw.c | 62 +++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 27 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix memory leakage in postgres_fdw's DirectModify code path. @ 2025-05-30 17:46 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2025-05-30 17:46 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix memory leakage in postgres_fdw's DirectModify code path. postgres_fdw tries to use PG_TRY blocks to ensure that it will eventually free the PGresult created by the remote modify command. However, it's fundamentally impossible for this scheme to work reliably when there's RETURNING data, because the query could fail in between invocations of postgres_fdw's DirectModify methods. There is at least one instance of exactly this situation in the regression tests, and the ensuing session-lifespan leak is visible under Valgrind. We can improve matters by using a memory context reset callback attached to the ExecutorState context. That ensures that the PGresult will be freed when the ExecutorState context is torn down, even if control never reaches postgresEndDirectModify. I have little faith that there aren't other potential PGresult leakages in the backend modules that use libpq. So I think it'd be a good idea to apply this concept universally by creating infrastructure that attaches a reset callback to every PGresult generated in the backend. However, that seems too invasive for v18 at this point, let alone the back branches. So for the moment, apply this narrow fix that just makes DirectModify safe. I have a patch in the queue for the more general idea, but it will have to wait for v19. Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com> Discussion: https://postgr.es/m/2976982.1748049023@sss.pgh.pa.us Backpatch-through: 13 Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/2b92dc4eeb51a1b66641718e072b96b63659448c Modified Files -------------- contrib/postgres_fdw/postgres_fdw.c | 62 +++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 27 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix memory leakage in postgres_fdw's DirectModify code path. @ 2025-05-30 17:46 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2025-05-30 17:46 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix memory leakage in postgres_fdw's DirectModify code path. postgres_fdw tries to use PG_TRY blocks to ensure that it will eventually free the PGresult created by the remote modify command. However, it's fundamentally impossible for this scheme to work reliably when there's RETURNING data, because the query could fail in between invocations of postgres_fdw's DirectModify methods. There is at least one instance of exactly this situation in the regression tests, and the ensuing session-lifespan leak is visible under Valgrind. We can improve matters by using a memory context reset callback attached to the ExecutorState context. That ensures that the PGresult will be freed when the ExecutorState context is torn down, even if control never reaches postgresEndDirectModify. I have little faith that there aren't other potential PGresult leakages in the backend modules that use libpq. So I think it'd be a good idea to apply this concept universally by creating infrastructure that attaches a reset callback to every PGresult generated in the backend. However, that seems too invasive for v18 at this point, let alone the back branches. So for the moment, apply this narrow fix that just makes DirectModify safe. I have a patch in the queue for the more general idea, but it will have to wait for v19. Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com> Discussion: https://postgr.es/m/2976982.1748049023@sss.pgh.pa.us Backpatch-through: 13 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/232d8caeaaa6ef7e7dfdc1a349ac956690949076 Modified Files -------------- contrib/postgres_fdw/postgres_fdw.c | 62 +++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 27 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2025-05-30 17:46 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2025-05-30 17:46 pgsql: Fix memory leakage in postgres_fdw's DirectModify code path. Tom Lane <tgl@sss.pgh.pa.us> 2025-05-30 17:46 pgsql: Fix memory leakage in postgres_fdw's DirectModify code path. Tom Lane <tgl@sss.pgh.pa.us> 2025-05-30 17:46 pgsql: Fix memory leakage in postgres_fdw's DirectModify code path. Tom Lane <tgl@sss.pgh.pa.us> 2025-05-30 17:46 pgsql: Fix memory leakage in postgres_fdw's DirectModify code path. Tom Lane <tgl@sss.pgh.pa.us> 2025-05-30 17:46 pgsql: Fix memory leakage in postgres_fdw's DirectModify code path. Tom Lane <tgl@sss.pgh.pa.us> 2025-05-30 17:46 pgsql: Fix memory leakage in postgres_fdw's DirectModify code path. Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox