MIME-Version: 1.0
References: <CAEudQAp=AW5dJXrGLbC_aZg_9nOo=42W7uLDRONFQE-gcgnkgQ@mail.gmail.com>
 <68d6d810-4ef1-4f62-b47f-89b21501249b@app.fastmail.com> <obqvpv4yx57lbnfiqvy4pmnfi2l5wn6xzoq2ftytxa426pa226@2sc46frt5xtb>
 <3337507.1739376124@sss.pgh.pa.us>
In-Reply-To: <3337507.1739376124@sss.pgh.pa.us>
From: Ranier Vilela <ranier.vf@gmail.com>
Date: Wed, 12 Feb 2025 13:46:38 -0300
Message-ID: <CAEudQArD_nKSnYCNUZiPPsJ2tNXgRmLbXGSOrH1vpOF_XtP0Vg@mail.gmail.com>
Subject: Re: Small memory fixes for pg_createsubcriber
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>, Euler Taveira <euler@eulerto.com>, 
	pgsql-hackers <pgsql-hackers@postgresql.org>
Content-Type: multipart/mixed; boundary="000000000000fde916062df4afd5"
Archived-At: <https://www.postgresql.org/message-id/CAEudQArD_nKSnYCNUZiPPsJ2tNXgRmLbXGSOrH1vpOF_XtP0Vg%40mail.gmail.com>
Precedence: bulk

--000000000000fde916062df4afd5
Content-Type: multipart/alternative; boundary="000000000000fde915062df4afd3"

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

Hi.

Em qua., 12 de fev. de 2025 =C3=A0s 13:02, Tom Lane <tgl@sss.pgh.pa.us> esc=
reveu:

> Andres Freund <andres@anarazel.de> writes:
> > On 2025-02-11 13:32:32 -0300, Euler Taveira wrote:
> >> There is no bug. They are the same behind the scenes.
>
> > That *is* a bug. On windows the allocator that a shared library (i.e.
> libpq)
> > uses, may *not* be the same as the one that an executable
> > (i.e. pg_createsubscriber).  It's not correct to free memory allocated
> in a
> > shared library just with free, it has to go through the library's free.
>
> Indeed.  This is particularly pernicious because it will work even on
> Windows under common scenarios (which no doubt explains the lack of
> field reports).  From the last time we discussed this [1]:
>
>     It seems to work fine as long as a debug-readline is paired with a
>     debug-psql or a release-readline is paired with a release-psql.
>
> I wish we had some way to detect misuses automatically ...
>
Unfortunately, for now, there is only manual search.

In fact, after your post, I did a new search and found two other
occurrences.
(src/bin/pg_amcheck/pg_amcheck.c)

One fixed in the patch attached.
Another *dat->amcheck_schema* is it not worth the effort,
since the memory is not released at any point?

Trivial patch attached.

best regards,
Ranier Vilela

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

<div dir=3D"ltr"><div dir=3D"ltr">Hi.</div><br><div class=3D"gmail_quote gm=
ail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">Em qua., 12 de f=
ev. de 2025 =C3=A0s 13:02, Tom Lane &lt;<a href=3D"mailto:tgl@sss.pgh.pa.us=
">tgl@sss.pgh.pa.us</a>&gt; escreveu:<br></div><blockquote class=3D"gmail_q=
uote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,2=
04);padding-left:1ex">Andres Freund &lt;<a href=3D"mailto:andres@anarazel.d=
e" target=3D"_blank">andres@anarazel.de</a>&gt; writes:<br>
&gt; On 2025-02-11 13:32:32 -0300, Euler Taveira wrote:<br>
&gt;&gt; There is no bug. They are the same behind the scenes.<br>
<br>
&gt; That *is* a bug. On windows the allocator that a shared library (i.e. =
libpq)<br>
&gt; uses, may *not* be the same as the one that an executable<br>
&gt; (i.e. pg_createsubscriber).=C2=A0 It&#39;s not correct to free memory =
allocated in a<br>
&gt; shared library just with free, it has to go through the library&#39;s =
free.<br>
<br>
Indeed.=C2=A0 This is particularly pernicious because it will work even on<=
br>
Windows under common scenarios (which no doubt explains the lack of<br>
field reports).=C2=A0 From the last time we discussed this [1]:<br>
<br>
=C2=A0 =C2=A0 It seems to work fine as long as a debug-readline is paired w=
ith a<br>
=C2=A0 =C2=A0 debug-psql or a release-readline is paired with a release-psq=
l.<br>
<br>
I wish we had some way to detect misuses automatically ...<br></blockquote>=
<div>Unfortunately, for now, there is only manual search.</div><div>=C2=A0<=
/div><div>In fact, after your post, I did a new search and found two other =
occurrences.</div><div>(src/bin/pg_amcheck/pg_amcheck.c)</div><div><br></di=
v><div>One fixed in the patch attached.</div><div>Another *dat-&gt;amcheck_=
schema* is it not worth the effort,=C2=A0</div><div>since the memory is not=
 released at any point?</div><div><br></div><div>Trivial patch attached.</d=
iv><div><br></div><div>best regards,</div><div>Ranier Vilela</div></div></d=
iv>

--000000000000fde915062df4afd3--
--000000000000fde916062df4afd5
Content-Type: application/octet-stream; 
	name="fix-api-misuse-pg_amcheck.patch"
Content-Disposition: attachment; filename="fix-api-misuse-pg_amcheck.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_m7257uzw0>
X-Attachment-Id: f_m7257uzw0

ZGlmZiAtLWdpdCBhL3NyYy9iaW4vcGdfYW1jaGVjay9wZ19hbWNoZWNrLmMgYi9zcmMvYmluL3Bn
X2FtY2hlY2svcGdfYW1jaGVjay5jCmluZGV4IGM1ZWMyNWJlMjIuLmRkYThiMWI4YWQgMTAwNjQ0
Ci0tLSBhL3NyYy9iaW4vcGdfYW1jaGVjay9wZ19hbWNoZWNrLmMKKysrIGIvc3JjL2Jpbi9wZ19h
bWNoZWNrL3BnX2FtY2hlY2suYwpAQCAtNTYwLDcgKzU2MCw3IEBAIG1haW4oaW50IGFyZ2MsIGNo
YXIgKmFyZ3ZbXSkKIAogCQkJZXhlY3V0ZUNvbW1hbmQoY29ubiwgaW5zdGFsbF9zcWwsIG9wdHMu
ZWNobyk7CiAJCQlwZnJlZShpbnN0YWxsX3NxbCk7Ci0JCQlwZnJlZShzY2hlbWEpOworCQkJUFFm
cmVlbWVtKHNjaGVtYSk7CiAJCX0KIAogCQkvKgo=
--000000000000fde916062df4afd5--