Re: Fix memleaks and error handling in jsonb_plpython - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Fix memleaks and error handling in jsonb_plpython
Date
Msg-id 20190305034522.GD3156@paquier.xyz
Whole thread Raw
In response to Fix memleaks and error handling in jsonb_plpython  (Nikita Glukhov <n.gluhov@postgrespro.ru>)
Responses Re: Fix memleaks and error handling in jsonb_plpython
List 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

Attachment

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: libpq debug log
Next
From: Amit Langote
Date:
Subject: Re: Inheriting table AMs for partitioned tables