On Wed, Feb 22, 2017 at 1:27 PM, Michael Paquier <michael.paqu...@gmail.com> wrote: > On Wed, Feb 22, 2017 at 8:39 AM, Fujii Masao <masao.fu...@gmail.com> wrote: >> On Wed, Feb 22, 2017 at 6:57 AM, Michael Paquier >> <michael.paqu...@gmail.com> wrote: >>> On Wed, Feb 22, 2017 at 4:12 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: >>>> Fujii Masao <fu...@postgresql.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? > > Thanks for the patch. That looks good to me.
Petr and Michael, Thanks for the review! Pushed. Regards, -- Fujii Masao -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
