Re: pgsql: Fix connection leak in DROP SUBSCRIPTION command.

From: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Fujii Masao <fujii(at)postgresql(dot)org>, pgsql-committers <pgsql-committers(at)postgresql(dot)org>
Subject: Re: pgsql: Fix connection leak in DROP SUBSCRIPTION command.
Date: 2017-02-21 23:39:21
Message-ID: CAHGQGwGcq1B1NaTTGAT8=TJkjvnOmhR6JjScVVr-7y8K6R8ypw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

On Wed, Feb 22, 2017 at 6:57 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Wed, Feb 22, 2017 at 4:12 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Fujii Masao <fujii(at)postgresql(dot)org> writes:
>>> Fix connection leak in DROP SUBSCRIPTION command.
>>> Previously the command forgot to close the connection to the publisher
>>> when it failed to drop the replication slot.
>>
>> If there's a bug here, this seems like an extremely unreliable way of
>> fixing it. What if an error gets thrown before you reach that ereport?
>>
>> In other words, this coding is assuming that the walrcv_command()
>> subroutine cannot throw an error,

Yes, but I agree that walrcv_command() may be changed in the future so that
an error is thrown and current coding is not reliable in that case.

>> which I would consider dangerous
>> even if it were a fixed subroutine. If it's a hook that's doing
>> unknown stuff, that seems a completely untenable assumption. You
>> really need either to hook the cleanup action into normal error
>> recovery, or to use a PG_TRY block.
>
> To be honest, I have thought about using PG_ENSURE_ERROR_CLEANUP()
> when seeing the thread. If other ERROR messages are generated in the
> future that the current fix would be unreliable.

What about the attached patch?

Regards,

--
Fujii Masao

Attachment Content-Type Size
bugfix.patch application/octet-stream 1.3 KB

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Petr Jelinek 2017-02-22 02:14:50 Re: pgsql: Fix connection leak in DROP SUBSCRIPTION command.
Previous Message Fujii Masao 2017-02-21 23:31:22 Re: pgsql: Make walsender always initialize the buffers.