From 0f3d5f2e388c3e34186b286d2cdd9c89f5bca3db Mon Sep 17 00:00:00 2001 From: Silvio Tomatis Date: Mon, 26 Mar 2018 13:50:34 +0200 Subject: [PATCH 1/9] Run memory leaks in travis using python3.4-dbg --- .travis.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.travis.yml b/.travis.yml index b3e486b..2cc61dd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,6 +11,10 @@ # # Here we execute the Python 3.6 tests; Python 3.4 and 3.5 tests are run by # cibuildwheel, inside the manylinux1 Docker image. + +# We also run tests for memory leaks (which require a debug build of python) +# using python3.4. Specifically we install python3.4-dbg, the only python3 +# runtime with debug support present on the travis-ci platform at the moment. # # See https://mail.python.org/pipermail/wheel-builders/2017-August/000285.html @@ -21,6 +25,12 @@ matrix: python: "3.6" services: - docker + - sudo: required + language: python + python: "3.4" + services: + - docker + env: RUN_DEBUG_PYTHON=true - os: osx osx_image: xcode8.3 @@ -34,6 +44,12 @@ env: install: - if [ $TRAVIS_OS_NAME = osx ]; then brew upgrade python; fi + - | + if [ $RUN_DEBUG_PYTHON = true ]; then + sudo apt-get install python3.4-dbg + virtualenv -p python3.4-dbg /tmp/env + . /tmp/env/bin/activate + fi - pip3 install -r requirements-test.txt script: From 86062b976c408b4c459a3bc7f4f551e15f37b48d Mon Sep 17 00:00:00 2001 From: Silvio Tomatis Date: Mon, 26 Mar 2018 14:18:45 +0200 Subject: [PATCH 2/9] Fix doctest travis run --- .travis.yml | 6 +++++- requirements-test.txt | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 2cc61dd..68228d0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -53,7 +53,11 @@ install: - pip3 install -r requirements-test.txt script: - - pip3 install . && pytest tests && cd docs; make doctest; cd - + - pip3 install . + - pytest tests + - pushd docs + - make doctest -e PYTHON=$(which python3) + - popd - | pip3 install cibuildwheel==0.6.0 cibuildwheel --output-dir wheelhouse diff --git a/requirements-test.txt b/requirements-test.txt index 90566ee..3523458 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -1,2 +1,3 @@ pytest >= 3 pytz +sphinx From 37762aa4b37608f73f72cd94fb261d4f32ed5f7f Mon Sep 17 00:00:00 2001 From: Silvio Tomatis Date: Mon, 26 Mar 2018 10:00:51 +0200 Subject: [PATCH 3/9] Add test for memory leak in case RawJSON constructor is called multiple times --- tests/test_refs_count.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_refs_count.py b/tests/test_refs_count.py index 6f99ef9..ccc1398 100644 --- a/tests/test_refs_count.py +++ b/tests/test_refs_count.py @@ -160,3 +160,15 @@ class Foo: del value rc1 = sys.gettotalrefcount() assert (rc1 - rc0) < THRESHOLD + + +@pytest.mark.skipif(not hasattr(sys, 'gettotalrefcount'), reason='Non-debug Python') +def test_rawjson_constructor(): + raw_json = rj.RawJSON('["foo", "bar"]') + rc0 = sys.gettotalrefcount() + for i in range(1000): + value = '"foobar"' + raw_json.__init__(value) + del value + rc1 = sys.gettotalrefcount() + assert (rc1 - rc0) < THRESHOLD From c22f2c97f0f387f805a1acfcf5dd52a683e062c9 Mon Sep 17 00:00:00 2001 From: Silvio Tomatis Date: Mon, 26 Mar 2018 10:04:17 +0200 Subject: [PATCH 4/9] Make sure that we don't leak memory if RawJSON constructor is called multiple times --- rapidjson.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/rapidjson.cpp b/rapidjson.cpp index 316e44e..6713722 100644 --- a/rapidjson.cpp +++ b/rapidjson.cpp @@ -441,13 +441,17 @@ RawJSON_init(RawJSON* self, PyObject* args, PyObject* kwds) "value", NULL }; - PyObject* value = NULL; + PyObject* value = NULL, *tmp; if (!PyArg_ParseTupleAndKeywords(args, kwds, "U", kwlist, &value)) return -1; - self->value = value; - Py_INCREF(self->value); + if (value) { + tmp = self->value; + Py_INCREF(value); + self->value = value; + Py_XDECREF(tmp); + } return 0; } From 1aa38998c241d85d4995d00dce3343f316b82303 Mon Sep 17 00:00:00 2001 From: Silvio Tomatis Date: Mon, 26 Mar 2018 15:36:57 +0200 Subject: [PATCH 5/9] Make doctests independent from local timezone --- docs/dumps.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/dumps.rst b/docs/dumps.rst index 0f074a9..b083d4f 100644 --- a/docs/dumps.rst +++ b/docs/dumps.rst @@ -294,9 +294,9 @@ .. doctest:: - >>> mode = DM_UNIX_TIME + >>> mode = DM_UNIX_TIME | DM_NAIVE_IS_UTC >>> dumps([now, now.date(), now.time()], datetime_mode=mode) - '[1472409071.084418,1472335200.0,73871.084418]' + '[1472409071.084418,1472342400.0,73871.084418]' >>> unixtime = float(dumps(now, datetime_mode=mode)) >>> datetime.fromtimestamp(unixtime, here) == now True @@ -306,9 +306,9 @@ .. doctest:: - >>> mode = DM_UNIX_TIME | DM_ONLY_SECONDS + >>> mode = DM_UNIX_TIME | DM_NAIVE_IS_UTC | DM_ONLY_SECONDS >>> dumps([now, now.date(), now.time()], datetime_mode=mode) - '[1472409071,1472335200,73871]' + '[1472409071,1472342400,73871]' It can be used combined with :data:`DM_SHIFT_TO_UTC` to obtain the timestamp of the corresponding UTC_ time: From d3bc12537949fa599e7764882ac13a116cb1650c Mon Sep 17 00:00:00 2001 From: Silvio Tomatis Date: Tue, 27 Mar 2018 10:03:40 +0200 Subject: [PATCH 6/9] Move the RawJSON initialization logic to the __new__ method --- rapidjson.cpp | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/rapidjson.cpp b/rapidjson.cpp index 6713722..2cd2187 100644 --- a/rapidjson.cpp +++ b/rapidjson.cpp @@ -434,28 +434,26 @@ RawJSON_dealloc(RawJSON* self) } -static int -RawJSON_init(RawJSON* self, PyObject* args, PyObject* kwds) +static PyObject * +RawJSON_new(PyTypeObject *type, PyObject *args, PyObject *kwds) { + PyObject * self; + self = type->tp_alloc(type, 0); static char * kwlist[] = { "value", NULL }; - PyObject* value = NULL, *tmp; + PyObject* value = NULL; if (!PyArg_ParseTupleAndKeywords(args, kwds, "U", kwlist, &value)) - return -1; + return NULL; - if (value) { - tmp = self->value; - Py_INCREF(value); - self->value = value; - Py_XDECREF(tmp); - } + ((RawJSON*)self)->value = value; - return 0; -} + Py_INCREF(value); + return self; +} static PyMemberDef RawJSON_members[] = { {"value", @@ -510,9 +508,9 @@ static PyTypeObject RawJSON_Type = { 0, /* tp_descr_get */ 0, /* tp_descr_set */ 0, /* tp_dictoffset */ - (initproc) RawJSON_init, /* tp_init */ + 0, /* tp_init */ 0, /* tp_alloc */ - PyType_GenericNew, /* tp_new */ + RawJSON_new, /* tp_new */ }; From 2937501ea952c8af736c3eff274f219121c09630 Mon Sep 17 00:00:00 2001 From: Silvio Tomatis Date: Tue, 27 Mar 2018 10:19:22 +0200 Subject: [PATCH 7/9] Remove bytecode files before running cibuildwheel --- .travis.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.travis.yml b/.travis.yml index 68228d0..2c38cc2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -58,6 +58,9 @@ script: - pushd docs - make doctest -e PYTHON=$(which python3) - popd + # cibuildwheel's docker might get confused and throw an ImportMismatchError, + # so we remove bytecode files before running it. + - find . -name __pycache__ -exec rm -rf {} + - | pip3 install cibuildwheel==0.6.0 cibuildwheel --output-dir wheelhouse From a45c006a19156ebf6743941a8702704539f8377a Mon Sep 17 00:00:00 2001 From: Silvio Tomatis Date: Tue, 27 Mar 2018 10:33:26 +0200 Subject: [PATCH 8/9] Add memory leak test for RawJSON.__new__ --- tests/test_refs_count.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test_refs_count.py b/tests/test_refs_count.py index ccc1398..4d9e921 100644 --- a/tests/test_refs_count.py +++ b/tests/test_refs_count.py @@ -172,3 +172,13 @@ def test_rawjson_constructor(): del value rc1 = sys.gettotalrefcount() assert (rc1 - rc0) < THRESHOLD + + +@pytest.mark.skipif(not hasattr(sys, 'gettotalrefcount'), reason='Non-debug Python') +def test_rawjson_new(): + rc0 = sys.gettotalrefcount() + for i in range(1000): + raw_json = rj.RawJSON('["foo", "bar"]') + del raw_json + rc1 = sys.gettotalrefcount() + assert (rc1 - rc0) < THRESHOLD From 5cc1441a0ca2ac64e036aacc3282e6a14683b317 Mon Sep 17 00:00:00 2001 From: Silvio Tomatis Date: Tue, 27 Mar 2018 10:53:15 +0200 Subject: [PATCH 9/9] Fix whitespace --- rapidjson.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rapidjson.cpp b/rapidjson.cpp index 2cd2187..fcd0d26 100644 --- a/rapidjson.cpp +++ b/rapidjson.cpp @@ -434,12 +434,12 @@ RawJSON_dealloc(RawJSON* self) } -static PyObject * -RawJSON_new(PyTypeObject *type, PyObject *args, PyObject *kwds) +static PyObject* +RawJSON_new(PyTypeObject* type, PyObject* args, PyObject* kwds) { - PyObject * self; + PyObject* self; self = type->tp_alloc(type, 0); - static char * kwlist[] = { + static char* kwlist[] = { "value", NULL }; @@ -448,7 +448,7 @@ RawJSON_new(PyTypeObject *type, PyObject *args, PyObject *kwds) if (!PyArg_ParseTupleAndKeywords(args, kwds, "U", kwlist, &value)) return NULL; - ((RawJSON*)self)->value = value; + ((RawJSON*) self)->value = value; Py_INCREF(value);