Re: Fix memleaks and error handling in jsonb_plpython

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>
Cc: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix memleaks and error handling in jsonb_plpython
Date: 2019-03-05 03:45:22
Message-ID: 20190305034522.GD3156@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Mar 01, 2019 at 05:24:39AM +0300, Nikita Glukhov wrote:
> Unfortunately, contrib/jsonb_plpython still contain a lot of problems in error
> handling that can lead to memory leaks:
> - not all Python function calls are checked for the success
> - not in all places PG exceptions are caught to release Python references
> But it seems that this errors can happen only in OOM case.
>
> Attached patch with the fix. Back-patch for PG11 is needed.

That looks right to me. Here are some comments.

One thing to be really careful of when using PG_TRY/PG_CATCH blocks is
that variables modified in the try block and then referenced in the
catch block need to be marked as volatile. If you don't do that, the
value when reaching the catch part is indeterminate.

With your patch the result variable used in two places of
PLyObject_FromJsonbContainer() is not marked as volatile. Similarly,
it seems that "items" in PLyMapping_ToJsonbValue() and "seq" in
"PLySequence_ToJsonbValue" should be volatile because they get changed
in the try loop, and referenced afterwards.

Another issue: in ltree_plpython we don't check the return state of
PyList_SetItem(), which we should complain about I think.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2019-03-05 03:59:56 Re: Inheriting table AMs for partitioned tables
Previous Message Robert Haas 2019-03-05 03:31:52 Re: libpq debug log