Re: [bug fix] ECPG: freeing memory for pgtypes crashes on Windows

From: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
To: "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>
Cc: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, MauMau <maumau307(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, "Michael Meskes (meskes(at)postgresql(dot)org)" <meskes(at)postgresql(dot)org>
Subject: Re: [bug fix] ECPG: freeing memory for pgtypes crashes on Windows
Date: 2018-06-18 06:37:34
Message-ID: CAEepm=0QKbwTmzecDtGTGprUh1VL5TGrd1RENKJy03egX29_wg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jun 12, 2018 at 2:04 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> writes:
>> One question I have is whether it is against project policy to
>> backport a new file and a new user-facing function.
>
> Given that this has been broken since forever, and there've been
> no complaints before now, I do not think the case for back-patching
> is strong enough to justify the problems it would cause. Just
> put it in v11 and be done.

Done.

> Also, this bit in the proposed documentation seems quite inappropriate:
>
> (This is a change from earlier releases of
> <productname>PostgreSQL</productname> ...
>
> We don't normally put in such comments at all, and if we do, we
> specify which version we're talking about. Two years from now
> this'd be totally confusing. I'd just make it read
>
> (This is important only on Windows, where ...

Done.

On Tue, Jun 12, 2018 at 1:09 PM, Tsunakawa, Takayuki
<tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com> wrote:
>> From: Thomas Munro [mailto:thomas(dot)munro(at)enterprisedb(dot)com]
>> Why is it OK that we do "free(outp_sqlda)" having got that pointer from
>> a statement like "exec sql fetch 1 from mycur1 into descriptor outp_sqlda"?
>> Isn't that memory allocated by libecpg.dll?
>
> My colleague is now preparing a patch for that, which adds a function ECPGFreeSQLDA() in libecpg.so. That thread is here:
>
> https://www.postgresql.org/message-id/25C1C6B2E7BE044889E4FE8643A58BA963A42097@G01JPEXMBKW03

Thanks. I will follow up on that thread.

> I want some remedy for older releases. Our customer worked around this problem by getting a libpq connection in their ECPG application and calling PQfreemem(). That's an ugly kludge, and I don't want other users to follow it.
>
> I don't see a problem with back-patching as-is, because existing users who just call free() or don't call free() won't be affected. I think that most serious applications somehow state their supported minor releases like "this application supports (or is certified against) PostgreSQL 10.5 or later", just like other applications support "RHEL 6.2 or later" or "Windows XP Sp2 or later."

If there is a consensus that we should do that then I'll back-patch,
but so far no one else has spoken up in support.

--
Thomas Munro
http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tsunakawa, Takayuki 2018-06-18 07:25:13 RE: [bug fix] ECPG: freeing memory for pgtypes crashes on Windows
Previous Message Michael Paquier 2018-06-18 06:02:00 Re: Partitioning with temp tables is broken