MIME-Version: 1.0
References: 
 <CA+fm-RMopta1Dmq8udiU5sp+zwTvhUf4+xfbr3rZDfczH+p-xw@mail.gmail.com>
 <DJ1FOVN3MWNT.1ZQ7NHQAVJPXY@partin.io>
In-Reply-To: <DJ1FOVN3MWNT.1ZQ7NHQAVJPXY@partin.io>
From: Baji Shaik <baji.pgdev@gmail.com>
Date: Fri, 5 Jun 2026 18:44:59 -0500
Message-ID: 
 <CA+fm-RMwUuX7SimgfHZ1o3=RYWQT7wC+KC2+uAgbPqHij25mug@mail.gmail.com>
Subject: Re: [PATCH] Fix memory leak in pgstat_progress_parallel_incr_param()
To: Tristan Partin <tristan@partin.io>
Cc: bertranddrouvot.pg@gmail.com, pgsql-hackers@lists.postgresql.org
Content-Type: multipart/mixed; boundary="00000000000060ad6106538a401c"
Archived-At: 
 <https://www.postgresql.org/message-id/CA%2Bfm-RMwUuX7SimgfHZ1o3%3DRYWQT7wC%2BKC2%2BuAgbPqHij25mug%40mail.gmail.com>
Precedence: bulk

--00000000000060ad6106538a401c
Content-Type: multipart/alternative; boundary="00000000000060ad5f06538a401a"

--00000000000060ad5f06538a401a
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Jun 5, 2026 at 4:29=E2=80=AFPM Tristan Partin <tristan@partin.io> w=
rote:

> This looks pretty reasonable to me. Nice find. Did you think about
> keeping the code path as is and just removing the first initStringInfo()
> call? Removing the allocation per progress message seems like a good
> idea to me. Maybe you could separate this change into two patches. One
> to fix the memory leak and another to remove the allocation per message.
> A committer could then decide for themselves if the second patch is
> worth committing.
>

Thank you for the review.  I hadn't thought of splitting it, but it's
a good idea.  I see f1889729dd3 itself is in PG17+, so the bug fix is
a backport candidate independently of the PG19 caller bb8dff9995f.

Patches attached:

  0001: drop the redundant initStringInfo() call (backport candidate)
  0002: allocate the static buffer once per process via
        pq_beginmessage_reuse / pq_endmessage_reuse, to avoid the
        per-call allocation (master only)

Thanks,
Baji Shaik

--00000000000060ad5f06538a401a
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">On Fri, Jun 5, 2026 at 4:29=E2=80=AFPM Tr=
istan Partin &lt;<a href=3D"mailto:tristan@partin.io">tristan@partin.io</a>=
&gt; wrote:</div><div class=3D"gmail_quote gmail_quote_container"><blockquo=
te class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px =
solid rgb(204,204,204);padding-left:1ex">
This looks pretty reasonable to me. Nice find. Did you think about <br>
keeping the code path as is and just removing the first initStringInfo() <b=
r>
call? Removing the allocation per progress message seems like a good <br>
idea to me. Maybe you could separate this change into two patches. One <br>
to fix the memory leak and another to remove the allocation per message. <b=
r>
A committer could then decide for themselves if the second patch is <br>
worth committing.<br></blockquote><div><br></div>Thank you for the review.=
=C2=A0 I hadn&#39;t thought of splitting it, but it&#39;s<br>a good idea.=
=C2=A0 I see f1889729dd3 itself is in PG17+, so the bug fix is<br>a backpor=
t candidate independently of the PG19 caller bb8dff9995f.<br><br>Patches at=
tached:<br><br>=C2=A0 0001: drop the redundant initStringInfo() call (backp=
ort candidate)<br>=C2=A0 0002: allocate the static buffer once per process =
via<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 pq_beginmessage_reuse / pq_endmessage_re=
use, to avoid the<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 per-call allocation (maste=
r only)<br><br>Thanks,<br>Baji Shaik</div></div>

--00000000000060ad5f06538a401a--

--00000000000060ad6106538a401c
Content-Type: application/octet-stream;
	name="0001-Fix-memory-leak-in-pgstat_progress_parallel_incr_par.patch"
Content-Disposition: attachment;
	filename="0001-Fix-memory-leak-in-pgstat_progress_parallel_incr_par.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mq1klbb50>
X-Attachment-Id: f_mq1klbb50

RnJvbSBjNzk3MmJjOGVlMzRlYzExMTQ0MGJmOTQ4YzBjZWJlNDc2ZmQ4Y2RiIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBCYWppIFNoYWlrIDxiYWppLnBnZGV2QGdtYWlsLmNvbT4KRGF0
ZTogRnJpLCA1IEp1biAyMDI2IDE4OjQxOjQ3IC0wNTAwClN1YmplY3Q6IFtQQVRDSCAxLzJdIEZp
eCBtZW1vcnkgbGVhayBpbiBwZ3N0YXRfcHJvZ3Jlc3NfcGFyYWxsZWxfaW5jcl9wYXJhbSgpCgpX
aGVuIGNhbGxlZCBmcm9tIGEgcGFyYWxsZWwgd29ya2VyLCBwZ3N0YXRfcHJvZ3Jlc3NfcGFyYWxs
ZWxfaW5jcl9wYXJhbSgpCmNhbGxzIGluaXRTdHJpbmdJbmZvKCkgb24gYSBzdGF0aWMgU3RyaW5n
SW5mb0RhdGEgYW5kIHRoZW4gaW1tZWRpYXRlbHkKY2FsbHMgcHFfYmVnaW5tZXNzYWdlKCksIHdo
aWNoIGNhbGxzIGluaXRTdHJpbmdJbmZvKCkgYWdhaW4uICBUaGUgc2Vjb25kCmNhbGwgb3Zlcndy
aXRlcyBidWYtPmRhdGEgd2l0aCBhIGZyZXNobHkgcGFsbG9jJ2QgYnVmZmVyLCBvcnBoYW5pbmcg
dGhlCmZpcnN0IG9uZS4gIHBxX2VuZG1lc3NhZ2UoKSB0aGVuIGZyZWVzIG9ubHkgdGhlIHNlY29u
ZCBidWZmZXIsIHNvIGVhY2gKY2FsbCBsZWFrcyB+MSBrQiBpbnRvIHRoZSBwZXItd29ya2VyIG1l
bW9yeSBjb250ZXh0LgoKRml4IGJ5IHJlbW92aW5nIHRoZSByZWR1bmRhbnQgaW5pdFN0cmluZ0lu
Zm8oKSBjYWxsLgoKT3ZlcnNpZ2h0IG9mIGYxODg5NzI5ZGQzICgiQWRkIG5ldyBwYXJhbGxlbCBt
ZXNzYWdlIHR5cGUgdG8gcHJvZ3Jlc3MKcmVwb3J0aW5nIikuCgpBdXRob3I6IEJhamkgU2hhaWsg
PGJhamkucGdkZXZAZ21haWwuY29tPgotLS0KIHNyYy9iYWNrZW5kL3V0aWxzL2FjdGl2aXR5L2Jh
Y2tlbmRfcHJvZ3Jlc3MuYyB8IDIgLS0KIDEgZmlsZSBjaGFuZ2VkLCAyIGRlbGV0aW9ucygtKQoK
ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3V0aWxzL2FjdGl2aXR5L2JhY2tlbmRfcHJvZ3Jlc3Mu
YyBiL3NyYy9iYWNrZW5kL3V0aWxzL2FjdGl2aXR5L2JhY2tlbmRfcHJvZ3Jlc3MuYwppbmRleCBi
MDM1OTc3MWRlNS4uNmQyMDQ5MTA1YWIgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL3V0aWxzL2Fj
dGl2aXR5L2JhY2tlbmRfcHJvZ3Jlc3MuYworKysgYi9zcmMvYmFja2VuZC91dGlscy9hY3Rpdml0
eS9iYWNrZW5kX3Byb2dyZXNzLmMKQEAgLTEwMCw4ICsxMDAsNiBAQCBwZ3N0YXRfcHJvZ3Jlc3Nf
cGFyYWxsZWxfaW5jcl9wYXJhbShpbnQgaW5kZXgsIGludDY0IGluY3IpCiAJewogCQlzdGF0aWMg
U3RyaW5nSW5mb0RhdGEgcHJvZ3Jlc3NfbWVzc2FnZTsKIAotCQlpbml0U3RyaW5nSW5mbygmcHJv
Z3Jlc3NfbWVzc2FnZSk7Ci0KIAkJcHFfYmVnaW5tZXNzYWdlKCZwcm9ncmVzc19tZXNzYWdlLCBQ
cU1zZ19Qcm9ncmVzcyk7CiAJCXBxX3NlbmRpbnQzMigmcHJvZ3Jlc3NfbWVzc2FnZSwgaW5kZXgp
OwogCQlwcV9zZW5kaW50NjQoJnByb2dyZXNzX21lc3NhZ2UsIGluY3IpOwotLSAKMi41MC4xIChB
cHBsZSBHaXQtMTU1KQoK
--00000000000060ad6106538a401c
Content-Type: application/octet-stream;
	name="0002-Allocate-progress-message-buffer-once-per-parallel-w.patch"
Content-Disposition: attachment;
	filename="0002-Allocate-progress-message-buffer-once-per-parallel-w.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mq1klbba1>
X-Attachment-Id: f_mq1klbba1

RnJvbSA5NGUxY2Q2Mzk1NDYxNmY4MTk1Mzk1MGFhZjcxMmZhMmFjZjc4OTEzIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBCYWppIFNoYWlrIDxiYWppLnBnZGV2QGdtYWlsLmNvbT4KRGF0
ZTogRnJpLCA1IEp1biAyMDI2IDE4OjQyOjA4IC0wNTAwClN1YmplY3Q6IFtQQVRDSCAyLzJdIEFs
bG9jYXRlIHByb2dyZXNzIG1lc3NhZ2UgYnVmZmVyIG9uY2UgcGVyIHBhcmFsbGVsIHdvcmtlcgoK
VGhlIHN0YXRpYyBTdHJpbmdJbmZvRGF0YSBkZWNsYXJlZCBpbiBwZ3N0YXRfcHJvZ3Jlc3NfcGFy
YWxsZWxfaW5jcl9wYXJhbSgpCndhcyBjbGVhcmx5IGludGVuZGVkIHRvIGJlIGFsbG9jYXRlZCBv
bmNlIHBlciBwcm9jZXNzIGFuZCByZXVzZWQgYWNyb3NzCmNhbGxzLCBidXQgdGhlIGJ1Z2d5IGRv
dWJsZS1pbml0U3RyaW5nSW5mbygpIHBhdHRlcm4gKGZpeGVkIGluIHRoZQpwcmV2aW91cyBjb21t
aXQpIGRlZmVhdGVkIHRoYXQgaW50ZW50OiBlYWNoIGNhbGwgc3RpbGwgYWxsb2NhdGVkIGFuZApm
cmVlZCBhIGZyZXNoIGJ1ZmZlciB2aWEgcHFfYmVnaW5tZXNzYWdlKCkgLyBwcV9lbmRtZXNzYWdl
KCkuCgpSZXN0b3JlIHRoZSBvcmlnaW5hbCBpbnRlbnQgYnkgaW5pdGlhbGl6aW5nIHRoZSBzdGF0
aWMgYnVmZmVyIG9ubHkgb24KdGhlIGZpcnN0IGNhbGwgYW5kIHN3aXRjaGluZyB0byBwcV9iZWdp
bm1lc3NhZ2VfcmV1c2UoKSBhbmQKcHFfZW5kbWVzc2FnZV9yZXVzZSgpLCB3aGljaCByZXNldCBh
bmQgcmV1c2UgdGhlIGV4aXN0aW5nIGJ1ZmZlcgp3aXRob3V0IChyZSlhbGxvY2F0aW5nLgoKQXV0
aG9yOiBCYWppIFNoYWlrIDxiYWppLnBnZGV2QGdtYWlsLmNvbT4KLS0tCiBzcmMvYmFja2VuZC91
dGlscy9hY3Rpdml0eS9iYWNrZW5kX3Byb2dyZXNzLmMgfCAxOCArKysrKysrKysrKysrKystLS0K
IDEgZmlsZSBjaGFuZ2VkLCAxNSBpbnNlcnRpb25zKCspLCAzIGRlbGV0aW9ucygtKQoKZGlmZiAt
LWdpdCBhL3NyYy9iYWNrZW5kL3V0aWxzL2FjdGl2aXR5L2JhY2tlbmRfcHJvZ3Jlc3MuYyBiL3Ny
Yy9iYWNrZW5kL3V0aWxzL2FjdGl2aXR5L2JhY2tlbmRfcHJvZ3Jlc3MuYwppbmRleCA2ZDIwNDkx
MDVhYi4uMDQ4Mzc0MWE4MGUgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL3V0aWxzL2FjdGl2aXR5
L2JhY2tlbmRfcHJvZ3Jlc3MuYworKysgYi9zcmMvYmFja2VuZC91dGlscy9hY3Rpdml0eS9iYWNr
ZW5kX3Byb2dyZXNzLmMKQEAgLTk5LDExICs5OSwyMyBAQCBwZ3N0YXRfcHJvZ3Jlc3NfcGFyYWxs
ZWxfaW5jcl9wYXJhbShpbnQgaW5kZXgsIGludDY0IGluY3IpCiAJaWYgKElzUGFyYWxsZWxXb3Jr
ZXIoKSkKIAl7CiAJCXN0YXRpYyBTdHJpbmdJbmZvRGF0YSBwcm9ncmVzc19tZXNzYWdlOwotCi0J
CXBxX2JlZ2lubWVzc2FnZSgmcHJvZ3Jlc3NfbWVzc2FnZSwgUHFNc2dfUHJvZ3Jlc3MpOworCQlz
dGF0aWMgYm9vbCBwcm9ncmVzc19tZXNzYWdlX2luaXRpYWxpemVkID0gZmFsc2U7CisKKwkJLyoK
KwkJICogSW5pdGlhbGl6ZSB0aGUgbWVzc2FnZSBidWZmZXIgb25jZSBwZXIgcHJvY2VzczsgcHFf
YmVnaW5tZXNzYWdlX3JldXNlKCkKKwkJICogYW5kIHBxX2VuZG1lc3NhZ2VfcmV1c2UoKSByZXNl
dCBhbmQgcmV1c2UgaXQgb24gZWFjaCBjYWxsIHRvIGF2b2lkCisJCSAqIHBhbGxvYyBvdmVyaGVh
ZC4KKwkJICovCisJCWlmICghcHJvZ3Jlc3NfbWVzc2FnZV9pbml0aWFsaXplZCkKKwkJeworCQkJ
aW5pdFN0cmluZ0luZm8oJnByb2dyZXNzX21lc3NhZ2UpOworCQkJcHJvZ3Jlc3NfbWVzc2FnZV9p
bml0aWFsaXplZCA9IHRydWU7CisJCX0KKworCQlwcV9iZWdpbm1lc3NhZ2VfcmV1c2UoJnByb2dy
ZXNzX21lc3NhZ2UsIFBxTXNnX1Byb2dyZXNzKTsKIAkJcHFfc2VuZGludDMyKCZwcm9ncmVzc19t
ZXNzYWdlLCBpbmRleCk7CiAJCXBxX3NlbmRpbnQ2NCgmcHJvZ3Jlc3NfbWVzc2FnZSwgaW5jcik7
Ci0JCXBxX2VuZG1lc3NhZ2UoJnByb2dyZXNzX21lc3NhZ2UpOworCQlwcV9lbmRtZXNzYWdlX3Jl
dXNlKCZwcm9ncmVzc19tZXNzYWdlKTsKIAl9CiAJZWxzZQogCQlwZ3N0YXRfcHJvZ3Jlc3NfaW5j
cl9wYXJhbShpbmRleCwgaW5jcik7Ci0tIAoyLjUwLjEgKEFwcGxlIEdpdC0xNTUpCgo=
--00000000000060ad6106538a401c--