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