Subject: Re: Jsonb transform for pl/python
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
 Aleksander Alekseev <a.alekseev@postgrespro.ru>,
 pgsql-hackers@lists.postgresql.org, Anthony Bykov <a.bykov@postgrespro.ru>
References: <20171025145100.3a19933f@anthony-24-g082ur>
 <20180112140425.41174bd3@anthony-24-g082ur>
 <151577178706.20515.9599911730425998398.pgcf@coridan.postgresql.org>
 <b6edf1ab-5440-8798-de08-eb816baf58ba@2ndquadrant.com>
 <f07b3955-e3e7-b410-b4fc-dae290774cf9@postgrespro.ru>
 <84a3eaf4-32f6-d2dd-7139-e9c5dd3fec2d@2ndquadrant.com>
 <c02881c2-26ca-dc53-137c-a1239aa9b8c0@2ndquadrant.com>
From: Nikita Glukhov <n.gluhov@postgrespro.ru>
Organization: Postgres Professional
Message-ID: <6001af16-b242-2527-bc7e-84b8a959163b@postgrespro.ru>
Date: Fri, 15 Jun 2018 14:10:04 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.8.0
MIME-Version: 1.0
In-Reply-To: <c02881c2-26ca-dc53-137c-a1239aa9b8c0@2ndquadrant.com>
Content-Type: multipart/mixed;
 boundary="------------35F346B7A3A7430D72423032"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------35F346B7A3A7430D72423032
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

On 28.03.2018 15:43, Peter Eisentraut wrote:

> On 3/21/18 18:50, Peter Eisentraut wrote:
>> On 3/12/18 11:26, Nikita Glukhov wrote:
>>> I have reviewed this patch.  Attached new 6th version of the patch with
>>> v5-v6 delta-patch.
>> Thanks for the update.  I'm working on committing this.
> Committed.
>
> Everything seemed to work well.  I just did a bit of cosmetic cleanups.
> I did a fair amount of tweaking on the naming of functions, the
> extensions and library names, etc., to make it look like the existing
> plpython transforms.  I also changed it so that the transform would
> support mappings and sequences other than dict and list.  I removed the
> non-ASCII test and the test with the huge numbers.


I found a memory leak in PLySequence_ToJsonbValue():
PyObject returned from PySequence_GetItem() is not released.

A bug can be easily reproduced using function roudtrip() from regression test:
SELECT roundtrip('[1,2,3]'::jsonb) FROM generate_series(1, 1000000);

Similar code in PLyMapping_ToJsonbValue() seems to be correct because
PyList_GetItem() and PyTuple_GetItem() return a borrowed reference.

Patch with fix is attached.

-- 
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


--------------35F346B7A3A7430D72423032
Content-Type: text/x-patch;
 name="0001-Fix-memory-leak-in-contrib-jsonb_plpython.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="0001-Fix-memory-leak-in-contrib-jsonb_plpython.patch"

From 53ab2aa2f345d7c4f5924b2e327b2d94c6f2c765 Mon Sep 17 00:00:00 2001
From: Nikita Glukhov <n.gluhov@postgrespro.ru>
Date: Fri, 15 Jun 2018 02:58:40 +0300
Subject: [PATCH] Fix memory leak in contrib/jsonb_plpython

---
 contrib/jsonb_plpython/jsonb_plpython.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/contrib/jsonb_plpython/jsonb_plpython.c b/contrib/jsonb_plpython/jsonb_plpython.c
index f752d6c..d6d6eeb 100644
--- a/contrib/jsonb_plpython/jsonb_plpython.c
+++ b/contrib/jsonb_plpython/jsonb_plpython.c
@@ -308,6 +308,8 @@ PLySequence_ToJsonbValue(PyObject *obj, JsonbParseState **jsonb_state)
 		PyObject   *value = PySequence_GetItem(obj, i);
 
 		(void) PLyObject_ToJsonbValue(value, jsonb_state, true);
+
+		Py_XDECREF(value);
 	}
 
 	return pushJsonbValue(jsonb_state, WJB_END_ARRAY, NULL);
-- 
2.7.4


--------------35F346B7A3A7430D72423032--