Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1vvHp6-00C4LV-0e for pgsql-bugs@arkaria.postgresql.org; Wed, 25 Feb 2026 16:33:16 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1vvHp5-007I2E-0P for pgsql-bugs@arkaria.postgresql.org; Wed, 25 Feb 2026 16:33:15 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1vvHp4-007I24-0f for pgsql-bugs@lists.postgresql.org; Wed, 25 Feb 2026 16:33:14 +0000 Received: from sender2-pp-e104.zoho.in ([169.148.134.104]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.98.2) (envelope-from ) id 1vvHoz-0000000194f-2UuG for pgsql-bugs@lists.postgresql.org; Wed, 25 Feb 2026 16:33:13 +0000 ARC-Seal: i=1; a=rsa-sha256; t=1772037178; cv=none; d=zohomail.in; s=zohoarc; b=ZMMutiX/KevtMD12jR012zRYLiDhAGdXBGyDWQmdS8OPHPQ/MpT1Pe8KWJEN6M77dedEPCa0VMput/IOkhraNyESbSe4naHoXyE9/aX5g9e6QR+OG+PeGgi+9jU2cKXhy09K9qWLF5jsb4INMZk1yQBw2qKCXpO6z5hDnzrluPY= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.in; s=zohoarc; t=1772037178; h=Content-Type:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To; bh=oqgAYutjy8K4G/MSzzdoh6aAuckVCgg0KdQaYqb5ctY=; b=OSUnqPVYykyPxYWF7lJQ6ZyszssJ7oLrytCTLok/kxhL3WCGz2MCMf+v5H+cHpS+nqfCqXVMh9QV4SXxZjEO4HtJVi6gX5YNlL7BZCovh6xNW7Z7gMQ/PMBZKVS4/b1cW1IYHPAxdZIX2yyyrC/g6AOfuYpdNE2cQR6ky61hzaQ= ARC-Authentication-Results: i=1; mx.zohomail.in; dkim=pass header.i=zohocorp.com; spf=pass smtp.mailfrom=vishal.g@zohocorp.com; dmarc=pass header.from= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1772037178; s=admin1; d=zohocorp.com; i=vishal.g@zohocorp.com; h=Date:Date:From:From:To:To:Cc:Cc:Message-Id:Message-Id:In-Reply-To:References:Subject:Subject:MIME-Version:Content-Type:Reply-To; bh=oqgAYutjy8K4G/MSzzdoh6aAuckVCgg0KdQaYqb5ctY=; b=BqQlP0BOVHfit/FiLQUFHVDhTVuQU2vpg9bS8BFqXOcEN23dpP46YUwNvcIRX9CX qzN+Cax5k28juQi7Xgrn8C+csMxRdCS9XMQ5zAYtnkp/1TiC9R/NJXbd3QwTlY0ieUi HC8ohv9Hag6FpdZqHRQenaeG1w1uYRr+sMLzTk6Q= Received: from mail.zoho.in by mx.zoho.in with SMTP id 1772037174812773.6267357660844; Wed, 25 Feb 2026 22:02:54 +0530 (IST) Date: Wed, 25 Feb 2026 22:02:54 +0530 From: Vishal Prasanna To: "kurodahayato" Cc: "pgsql-bugs" Message-Id: <19c95a57600.599bb79483132.9142801067257165882@zohocorp.com> In-Reply-To: References: <19c7623e882.4080fd5426212.311756747309556767@zohocorp.com> Subject: RE: [BUG] Assert failure in ReorderBufferReturnTXN during logical decoding due to leaked specinsert change MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_Part_288085_663630425.1772037174784" Importance: Medium User-Agent: Zoho Mail X-Mailer: Zoho Mail X-Zoho-Virus-Status: 1 X-Zoho-AV-Stamp: zmail-av-0.1.0.1.4.3/271.998.15 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------=_Part_288085_663630425.1772037174784 Content-Type: multipart/alternative; boundary="----=_Part_288086_1951371867.1772037174784" ------=_Part_288086_1951371867.1772037174784 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, Thanks for the response. > Anyway, I agreed that the leak could happen across all versions. > How about fixing the issue as shown below? Thought? > 1. Add an Assert() for PG16-. Yeah, Adding `Assert(txn->size =3D=3D 0)` to PG 14, 15, 16 would ensure eve= rything is cleaned up before freeing the `ReorderBufferTXN`. Consider adding this a= ssert as well. > 2. Make sure specinsert is released for all supported versions. In PG 14 to 17, `ReorderBufferReturnChange()` is used to free the `ReorderB= ufferChange`. In PG 18, this function was renamed to `ReorderBufferFreeChange()`. Refer: PG 14, 15, 16, 17: PG14-17-Fix-specinsert-leak-in-ReorderBufferProcessTXN-e= rror-path.patch PG 18: PG18-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch The above changes cover the `specinsert` cleanup in the error path for all = supported versions. > 3. Consider a workload that could reproduce on PG18 and master. > 3-1. If found, the test code can be added for all versions. > 3-2. Otherwise, the test code can be added for PG17-. Found a workload that can reproduce the issue across all supported versions= , except PG 14. For PG 14, since row_filter is not supported, so we can go with the 'public= ation does not exist' error instead. Refer: PG14-Test-specinsert-cleanup-in-ReorderBufferProcessTXN-error-path.p= atch For PG 15 - 18, using the row_filter option we can cause an error in the lo= gical decoder. Refer: PG15-18-Test-specinsert-cleanup-in-ReorderBufferProcessTXN-error-pat= h.patch >> Additional Suggestion:=20 >> Currently in `PG_CATCH` block, `specinsert` is only freed in the `ERRCOD= E_TRANSACTION_ROLLBACK` branch=20 >> for streaming or prepared transactions, via `ReorderBufferResetTXN()` at= line 2691.=20 >> Would it make sense to move the freeing of `specinsert` before the if/el= se branch,=20 >> so that it is always freed regardless of the error path? This would avoi= d duplication and ensure=20 >> that `specinsert` is always cleaned up.=20 > It looks OK for me. In this case an argument should be reduced from=20 > ReorderBufferResetTXN(), right? It is harmless because the function is a = static one. Yes, the `specinsert` is no longer needed in `ReorderBufferResetTXN()`. Upd= ated the patch where `specinsert` cleanup is now handled in the `PG_CATCH()` block of `ReorderBufferProcessTXN()`, so= it is always freed before the if/else branch. Refer: PG14-17-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch PG18-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch Regards, Vishal Prasanna Zoho=C2=A0Corporation ------=_Part_288086_1951371867.1772037174784 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable =
Hi,

Than= ks for the response.

> Anyway, I agreed tha= t the leak could happen across all versions.
>= How about fixing the issue as shown below? Thought?
=
> 1. Add an Assert() for PG16-.

Yeah, Adding `Assert(txn->size =3D=3D 0)` to PG 14, 15, 16 would e= nsure everything
is cleaned up before freeing the `ReorderBuf= ferTXN`. Consider adding this assert as well.
> 2. Make sure specinsert is released for all supported ver= sions.

In PG 14 to 17, `ReorderBufferReturnCha= nge()` is used to free the `ReorderBufferChange`.
In PG= 18, this function was renamed to `ReorderBufferFreeChange()`.

Refer:
PG 14, 15, 16, 17: PG14-17-Fix-= specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch
P= G 18: PG18-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch<= br>

The above changes cover the `specinsert` clean= up in the error path for all supported versions.

> 3. Consider a workload that could reproduce on PG18 an= d master.
> 3-1. If found, the test code can b= e added for all versions.
> 3-2. Otherwise, the test code = can be added for PG17-.

Found a workload that = can reproduce the issue across all supported versions, except PG 14.

For PG 14, since row_filter is not supported, s= o we can go with the 'publication does not exist' error instead.
<= div>Refer: PG14-Test-specinsert-cleanup-in-ReorderBufferProcessTXN-error-pa= th.patch

For PG 15 - 18, using the row_f= ilter option we can cause an error in the logical decoder.
Re= fer: PG15-18-Test-specinsert-cleanup-in-ReorderBufferProcessTXN-error-path.= patch

>> Additional Suggesti= on:
>> Currently in `PG_CATCH` block, `spe= cinsert` is only freed in the `ERRCODE_TRANSACTION_ROLLBACK` branch
>> for streaming or prepared transactions, via `R= eorderBufferResetTXN()` at line 2691.
>> W= ould it make sense to move the freeing of `specinsert` before the if/else b= ranch,
>> so that it is always freed regar= dless of the error path? This would avoid duplication and ensure
=
>> that `specinsert` is always cleaned up.

> It looks OK for me. In th= is case an argument should be reduced from
> = ReorderBufferResetTXN(), right? It is harmless because the function is a st= atic one.

Yes, the `specinsert` is no longer n= eeded in `ReorderBufferResetTXN()`. Updated the patch where `specinsert` cl= eanup
is now handled in the `PG_CATCH()` block of `ReorderBuf= ferProcessTXN()`, so it is always freed before the if/else branch.
Refer:
PG14-17-Fix-specinsert-leak-in-ReorderBufferProc= essTXN-error-path.patch
PG18-Fix-specinsert-leak-in-ReorderBu= fferProcessTXN-error-path.patch


Regards,
Vishal Prasanna
Zoho Corporation<= /span>


<= /body> ------=_Part_288086_1951371867.1772037174784-- ------=_Part_288085_663630425.1772037174784 Content-Type: application/octet-stream; name=PG14-17-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch Content-Transfer-Encoding: 7bit X-ZM_AttachId: 139913299747850000 Content-Disposition: attachment; filename=PG14-17-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch From 66f3e22b29a61a1817e5482b335fb387c873955b Mon Sep 17 00:00:00 2001 From: Vishal Prasanna Date: Wed, 25 Feb 2026 15:52:22 +0530 Subject: [PATCH] Fix specinsert leak in ReorderBufferProcessTXN error path --- .../replication/logical/reorderbuffer.c | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 4bd1f7af061..51ac4d04f25 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -2081,8 +2081,7 @@ static void ReorderBufferResetTXN(ReorderBuffer *rb, ReorderBufferTXN *txn, Snapshot snapshot_now, CommandId command_id, - XLogRecPtr last_lsn, - ReorderBufferChange *specinsert) + XLogRecPtr last_lsn) { /* Discard the changes that we just streamed */ ReorderBufferTruncateTXN(rb, txn, rbtxn_prepared(txn)); @@ -2090,13 +2089,6 @@ ReorderBufferResetTXN(ReorderBuffer *rb, ReorderBufferTXN *txn, /* Free all resources allocated for toast reconstruction */ ReorderBufferToastReset(rb, txn); - /* Return the spec insert change if it is not NULL */ - if (specinsert != NULL) - { - ReorderBufferReturnChange(rb, specinsert, true); - specinsert = NULL; - } - /* * For the streaming case, stop the stream and remember the command ID and * snapshot for the streaming run. @@ -2663,6 +2655,13 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn, if (using_subtxn) RollbackAndReleaseCurrentSubTransaction(); + /* Free the specinsert change before freeing the ReorderBufferTXN */ + if (specinsert != NULL) + { + ReorderBufferReturnChange(rb, specinsert, true); + specinsert = NULL; + } + /* * The error code ERRCODE_TRANSACTION_ROLLBACK indicates a concurrent * abort of the (sub)transaction we are streaming or preparing. We @@ -2689,8 +2688,7 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn, /* Reset the TXN so that it is allowed to stream remaining data. */ ReorderBufferResetTXN(rb, txn, snapshot_now, - command_id, prev_lsn, - specinsert); + command_id, prev_lsn); } else { -- 2.50.1 (Apple Git-155) ------=_Part_288085_663630425.1772037174784 Content-Type: application/octet-stream; name=PG14-Test-specinsert-cleanup-in-ReorderBufferProcessTXN-error-path.patch Content-Transfer-Encoding: 7bit X-ZM_AttachId: 139913299747870270 Content-Disposition: attachment; filename=PG14-Test-specinsert-cleanup-in-ReorderBufferProcessTXN-error-path.patch From 3b781f72bdd8c6a897b7c6b1265c1ad41581f21a Mon Sep 17 00:00:00 2001 From: Vishal Prasanna Date: Wed, 25 Feb 2026 20:39:54 +0530 Subject: [PATCH 1/2] Test specinsert cleanup in ReorderBufferProcessTXN error path --- src/test/subscription/t/100_bugs.pl | 40 ++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl index 235227c7727..c195fd33bce 100644 --- a/src/test/subscription/t/100_bugs.pl +++ b/src/test/subscription/t/100_bugs.pl @@ -6,7 +6,7 @@ use strict; use warnings; use PostgresNode; use TestLib; -use Test::More tests => 11; +use Test::More tests => 12; # Bug #15114 @@ -392,3 +392,41 @@ like( $node_publisher->safe_psql('postgres', "DROP DATABASE regress_db"); $node_publisher->stop('fast'); + +# Ensure logical decoder doesn't crash if error occurs +# while processing an INSERT ... ON CONFLICT statement +$node_publisher = get_new_node('logical_decoder'); +$node_publisher->init(allows_streaming => 'logical'); +$node_publisher->start; + +$node_publisher->safe_psql( + 'postgres', qq( + CREATE TABLE tab_upsert (a INT PRIMARY KEY, b INT); + SELECT * FROM pg_create_logical_replication_slot('upsert_slot', 'pgoutput'); + INSERT INTO tab_upsert (a, b) VALUES (1, 1) + ON CONFLICT(a) DO UPDATE SET b = excluded.b; +)); + +# Decode the changes without a publication and +# verify that the logical decoder doesn't crash. +($ret, $stdout, $stderr) = $node_publisher->psql( + 'postgres', qq( + SELECT * + FROM pg_logical_slot_peek_binary_changes( + 'upsert_slot', + NULL, + NULL, + 'proto_version', '1', + 'publication_names', 'pub_that_does_not_exist' + ); +)); + +ok( $stderr =~ qr/publication "pub_that_does_not_exist" does not exist/, + 'peek logical changes with non-existent publication throws error' +); + +# Clean up +$node_publisher->safe_psql('postgres', "SELECT pg_drop_replication_slot('upsert_slot')"); +$node_publisher->safe_psql('postgres', "DROP TABLE tab_upsert"); + +$node_publisher->stop('fast'); -- 2.50.1 (Apple Git-155) ------=_Part_288085_663630425.1772037174784 Content-Type: application/octet-stream; name=PG15-18-Test-specinsert-cleanup-in-ReorderBufferProcessTXN-error-path.patch Content-Transfer-Encoding: 7bit X-ZM_AttachId: 139913299747890250 Content-Disposition: attachment; filename=PG15-18-Test-specinsert-cleanup-in-ReorderBufferProcessTXN-error-path.patch From bb4d6db02d79828cc7665690a9272b71970ead65 Mon Sep 17 00:00:00 2001 From: Vishal Prasanna Date: Wed, 25 Feb 2026 15:17:25 +0530 Subject: [PATCH 1/2] Test specinsert cleanup in ReorderBufferProcessTXN error path --- src/test/subscription/t/100_bugs.pl | 41 +++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl index 50223054918..4344eb581da 100644 --- a/src/test/subscription/t/100_bugs.pl +++ b/src/test/subscription/t/100_bugs.pl @@ -605,4 +605,45 @@ $node_publisher->safe_psql('postgres', "DROP DATABASE regress_db"); $node_publisher->stop('fast'); +# Ensure logical decoder doesn't crash if error occurs +# while processing an INSERT ... ON CONFLICT statement. +$node_publisher = PostgreSQL::Test::Cluster->new('logical_decoder'); +$node_publisher->init(allows_streaming => 'logical'); +$node_publisher->start; + +# The publication row filter WHERE ((a / 0) > 0) will trigger a division by zero error. +$node_publisher->safe_psql( + 'postgres', qq( + CREATE TABLE tab_upsert (a INT PRIMARY KEY, b INT); + CREATE PUBLICATION pub_rowfilter_error FOR TABLE tab_upsert WHERE ((a / 0) > 0); + SELECT * FROM pg_create_logical_replication_slot('upsert_slot', 'pgoutput'); + INSERT INTO tab_upsert (a, b) VALUES (1, 1) + ON CONFLICT(a) DO UPDATE SET b = excluded.b; +)); + +# Decode the changes with a publication whose row filter causes a +# division by zero error, and verify that the logical decoder doesn't crash. +($ret, $stdout, $stderr) = $node_publisher->psql( + 'postgres', qq( + SELECT * + FROM pg_logical_slot_peek_binary_changes( + 'upsert_slot', + NULL, + NULL, + 'proto_version', '1', + 'publication_names', 'pub_rowfilter_error' + ); +)); + +ok( $stderr =~ qr/division by zero/, + 'peek logical changes with row filter causing division by zero throws error' +); + +# Clean up +$node_publisher->safe_psql('postgres', "SELECT pg_drop_replication_slot('upsert_slot')"); +$node_publisher->safe_psql('postgres', "DROP PUBLICATION pub_rowfilter_error"); +$node_publisher->safe_psql('postgres', "DROP TABLE tab_upsert"); + +$node_publisher->stop('fast'); + done_testing(); -- 2.50.1 (Apple Git-155) ------=_Part_288085_663630425.1772037174784 Content-Type: application/octet-stream; name=PG18-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch Content-Transfer-Encoding: 7bit X-ZM_AttachId: 139913299747910010 Content-Disposition: attachment; filename=PG18-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch From 3aaefccb77c0a47926ab8567ec28682c3ed136a0 Mon Sep 17 00:00:00 2001 From: Vishal Prasanna Date: Wed, 25 Feb 2026 15:29:50 +0530 Subject: [PATCH 2/2] Fix specinsert leak in ReorderBufferProcessTXN error path --- .../replication/logical/reorderbuffer.c | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 11139a910b8..5dce51f2ee4 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -2164,8 +2164,7 @@ static void ReorderBufferResetTXN(ReorderBuffer *rb, ReorderBufferTXN *txn, Snapshot snapshot_now, CommandId command_id, - XLogRecPtr last_lsn, - ReorderBufferChange *specinsert) + XLogRecPtr last_lsn) { /* Discard the changes that we just streamed */ ReorderBufferTruncateTXN(rb, txn, rbtxn_is_prepared(txn)); @@ -2173,13 +2172,6 @@ ReorderBufferResetTXN(ReorderBuffer *rb, ReorderBufferTXN *txn, /* Free all resources allocated for toast reconstruction */ ReorderBufferToastReset(rb, txn); - /* Return the spec insert change if it is not NULL */ - if (specinsert != NULL) - { - ReorderBufferFreeChange(rb, specinsert, true); - specinsert = NULL; - } - /* * For the streaming case, stop the stream and remember the command ID and * snapshot for the streaming run. @@ -2753,6 +2745,13 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn, if (using_subtxn) RollbackAndReleaseCurrentSubTransaction(); + /* Free the specinsert change before freeing the ReorderBufferTXN */ + if (specinsert != NULL) + { + ReorderBufferFreeChange(rb, specinsert, true); + specinsert = NULL; + } + /* * The error code ERRCODE_TRANSACTION_ROLLBACK indicates a concurrent * abort of the (sub)transaction we are streaming or preparing. We @@ -2786,8 +2785,7 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn, /* Reset the TXN so that it is allowed to stream remaining data. */ ReorderBufferResetTXN(rb, txn, snapshot_now, - command_id, prev_lsn, - specinsert); + command_id, prev_lsn); } else { -- 2.50.1 (Apple Git-155) ------=_Part_288085_663630425.1772037174784--