From 44b10a08cd1c925fcc2396bc87eac4c681e02911 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Tue, 13 Feb 2018 12:28:33 +0200 Subject: [PATCH 1/2] bpo-31787: Prevent refleaks when calling __init__() more than once (GH-3995) (cherry picked from commit d019bc8319ea35e93bf4baa38098ff1b57cd3ee5) --- Lib/test/test_asyncio/test_tasks.py | 14 +++++++++++ Lib/test/test_bz2.py | 11 +++++++++ Lib/test/test_descr.py | 18 ++++++++++++++ Lib/test/test_hashlib.py | 9 +++++++ Lib/test/test_lzma.py | 11 +++++++++ Lib/test/test_property.py | 11 +++++++++ .../2018-02-09-21-41-56.bpo-31787.owSZ2t.rst | 2 ++ Modules/_asynciomodule.c | 24 +++++++++++++++---- Modules/_bz2module.c | 2 +- Modules/_hashopenssl.c | 4 ++-- Modules/_lzmamodule.c | 2 +- Objects/descrobject.c | 10 ++++---- Objects/funcobject.c | 4 ++-- 13 files changed, 106 insertions(+), 16 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-02-09-21-41-56.bpo-31787.owSZ2t.rst diff --git a/Lib/test/test_asyncio/test_tasks.py b/Lib/test/test_asyncio/test_tasks.py index 42da1fa19548f1..f41160ba3222d3 100644 --- a/Lib/test/test_asyncio/test_tasks.py +++ b/Lib/test/test_asyncio/test_tasks.py @@ -2133,6 +2133,20 @@ class CTask_CFuture_Tests(BaseTaskTests, test_utils.TestCase): Task = getattr(tasks, '_CTask', None) Future = getattr(futures, '_CFuture', None) + @support.refcount_test + def test_refleaks_in_task___init__(self): + gettotalrefcount = support.get_attribute(sys, 'gettotalrefcount') + @asyncio.coroutine + def coro(): + pass + task = self.new_task(self.loop, coro()) + self.loop.run_until_complete(task) + refs_before = gettotalrefcount() + for i in range(100): + task.__init__(coro(), loop=self.loop) + self.loop.run_until_complete(task) + self.assertAlmostEqual(gettotalrefcount() - refs_before, 0, delta=10) + @unittest.skipUnless(hasattr(futures, '_CFuture'), 'requires the C _asyncio module') diff --git a/Lib/test/test_bz2.py b/Lib/test/test_bz2.py index eaa472a6ccda03..f340f2330c3dab 100644 --- a/Lib/test/test_bz2.py +++ b/Lib/test/test_bz2.py @@ -13,6 +13,7 @@ import sys from test.support import unlink import _compression +import sys try: import threading @@ -828,6 +829,16 @@ def test_failure(self): # Previously, a second call could crash due to internal inconsistency self.assertRaises(Exception, bzd.decompress, self.BAD_DATA * 30) + @support.refcount_test + def test_refleaks_in___init__(self): + gettotalrefcount = support.get_attribute(sys, 'gettotalrefcount') + bzd = BZ2Decompressor() + refs_before = gettotalrefcount() + for i in range(100): + bzd.__init__() + self.assertAlmostEqual(gettotalrefcount() - refs_before, 0, delta=10) + + class CompressDecompressTest(BaseTest): def testCompress(self): data = bz2.compress(self.TEXT) diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py index c5bff7718dc72b..0d33e9179308bf 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -1519,6 +1519,15 @@ def f(cls, arg): return (cls, arg) del cm.x self.assertNotHasAttr(cm, "x") + @support.refcount_test + def test_refleaks_in_classmethod___init__(self): + gettotalrefcount = support.get_attribute(sys, 'gettotalrefcount') + cm = classmethod(None) + refs_before = gettotalrefcount() + for i in range(100): + cm.__init__(None) + self.assertAlmostEqual(gettotalrefcount() - refs_before, 0, delta=10) + @support.impl_detail("the module 'xxsubtype' is internal") def test_classmethods_in_c(self): # Testing C-based class methods... @@ -1574,6 +1583,15 @@ class D(C): del sm.x self.assertNotHasAttr(sm, "x") + @support.refcount_test + def test_refleaks_in_staticmethod___init__(self): + gettotalrefcount = support.get_attribute(sys, 'gettotalrefcount') + sm = staticmethod(None) + refs_before = gettotalrefcount() + for i in range(100): + sm.__init__(None) + self.assertAlmostEqual(gettotalrefcount() - refs_before, 0, delta=10) + @support.impl_detail("the module 'xxsubtype' is internal") def test_staticmethods_in_c(self): # Testing C-based static methods... diff --git a/Lib/test/test_hashlib.py b/Lib/test/test_hashlib.py index 177143e01dc78f..751a713b3e8b64 100644 --- a/Lib/test/test_hashlib.py +++ b/Lib/test/test_hashlib.py @@ -165,6 +165,15 @@ def hash_constructors(self): constructors = self.constructors_to_test.values() return itertools.chain.from_iterable(constructors) + @support.refcount_test + def test_refleaks_in_hash___init__(self): + gettotalrefcount = support.get_attribute(sys, 'gettotalrefcount') + sha1_hash = c_hashlib.new('sha1') + refs_before = gettotalrefcount() + for i in range(100): + sha1_hash.__init__('sha1') + self.assertAlmostEqual(gettotalrefcount() - refs_before, 0, delta=10) + def test_hash_array(self): a = array.array("b", range(10)) for cons in self.hash_constructors: diff --git a/Lib/test/test_lzma.py b/Lib/test/test_lzma.py index d7a8576d512340..3dc2c1e7e3b779 100644 --- a/Lib/test/test_lzma.py +++ b/Lib/test/test_lzma.py @@ -4,6 +4,8 @@ import pathlib import pickle import random +import sys +from test import support import unittest from test.support import ( @@ -364,6 +366,15 @@ def test_pickle(self): with self.assertRaises(TypeError): pickle.dumps(LZMADecompressor(), proto) + @support.refcount_test + def test_refleaks_in_decompressor___init__(self): + gettotalrefcount = support.get_attribute(sys, 'gettotalrefcount') + lzd = LZMADecompressor() + refs_before = gettotalrefcount() + for i in range(100): + lzd.__init__() + self.assertAlmostEqual(gettotalrefcount() - refs_before, 0, delta=10) + class CompressDecompressFunctionTestCase(unittest.TestCase): diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py index 26b7d5283a2220..f6f8f5ed0e45ee 100644 --- a/Lib/test/test_property.py +++ b/Lib/test/test_property.py @@ -3,6 +3,7 @@ import sys import unittest +from test import support class PropertyBase(Exception): pass @@ -173,6 +174,16 @@ def spam(self): sub.__class__.spam.__doc__ = 'Spam' self.assertEqual(sub.__class__.spam.__doc__, 'Spam') + @support.refcount_test + def test_refleaks_in___init__(self): + gettotalrefcount = support.get_attribute(sys, 'gettotalrefcount') + fake_prop = property('fget', 'fset', 'fdel', 'doc') + refs_before = gettotalrefcount() + for i in range(100): + fake_prop.__init__('fget', 'fset', 'fdel', 'doc') + self.assertAlmostEqual(gettotalrefcount() - refs_before, 0, delta=10) + + # Issue 5890: subclasses of property do not preserve method __doc__ strings class PropertySub(property): """This is a subclass of property""" diff --git a/Misc/NEWS.d/next/Library/2018-02-09-21-41-56.bpo-31787.owSZ2t.rst b/Misc/NEWS.d/next/Library/2018-02-09-21-41-56.bpo-31787.owSZ2t.rst new file mode 100644 index 00000000000000..f0cde59d740f06 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-02-09-21-41-56.bpo-31787.owSZ2t.rst @@ -0,0 +1,2 @@ +Fixed refleaks of ``__init__()`` methods in various modules. +(Contributed by Oren Milman) diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 35632a6e6faacb..0d887ba802ca6d 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -132,6 +132,7 @@ future_schedule_callbacks(FutureObj *fut) return 0; } + static int future_init(FutureObj *fut, PyObject *loop) { @@ -139,6 +140,19 @@ future_init(FutureObj *fut, PyObject *loop) int is_true; _Py_IDENTIFIER(get_debug); + // Same to FutureObj_clear() but not clearing fut->dict + Py_CLEAR(fut->fut_loop); + Py_CLEAR(fut->fut_callback0); + Py_CLEAR(fut->fut_context0); + Py_CLEAR(fut->fut_callbacks); + Py_CLEAR(fut->fut_result); + Py_CLEAR(fut->fut_exception); + Py_CLEAR(fut->fut_source_tb); + + fut->fut_state = STATE_PENDING; + fut->fut_log_tb = 0; + fut->fut_blocking = 0; + if (loop == Py_None) { loop = _PyObject_CallNoArg(asyncio_get_event_loop); if (loop == NULL) { @@ -148,7 +162,7 @@ future_init(FutureObj *fut, PyObject *loop) else { Py_INCREF(loop); } - Py_XSETREF(fut->fut_loop, loop); + fut->fut_loop = loop; res = _PyObject_CallMethodId(fut->fut_loop, &PyId_get_debug, NULL); if (res == NULL) { @@ -160,13 +174,13 @@ future_init(FutureObj *fut, PyObject *loop) return -1; } if (is_true) { - Py_XSETREF(fut->fut_source_tb, _PyObject_CallNoArg(traceback_extract_stack)); + fut->fut_source_tb = _PyObject_CallNoArg(traceback_extract_stack); if (fut->fut_source_tb == NULL) { return -1; } } - Py_XSETREF(fut->fut_callbacks, PyList_New(0)); + fut->fut_callbacks = PyList_New(0); if (fut->fut_callbacks == NULL) { return -1; } @@ -1336,12 +1350,12 @@ _asyncio_Task___init___impl(TaskObj *self, PyObject *coro, PyObject *loop) return -1; } - self->task_fut_waiter = NULL; + Py_CLEAR(self->task_fut_waiter); self->task_must_cancel = 0; self->task_log_destroy_pending = 1; Py_INCREF(coro); - self->task_coro = coro; + Py_XSETREF(self->task_coro, coro); if (task_call_step_soon(self, NULL)) { return -1; diff --git a/Modules/_bz2module.c b/Modules/_bz2module.c index 5cea42cc6b2f81..9c5a631eff45f3 100644 --- a/Modules/_bz2module.c +++ b/Modules/_bz2module.c @@ -663,7 +663,7 @@ _bz2_BZ2Decompressor___init___impl(BZ2Decompressor *self) self->bzs_avail_in_real = 0; self->input_buffer = NULL; self->input_buffer_size = 0; - self->unused_data = PyBytes_FromStringAndSize(NULL, 0); + Py_XSETREF(self->unused_data, PyBytes_FromStringAndSize(NULL, 0)); if (self->unused_data == NULL) goto error; diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 8670fbd4572d02..8f6185ccb73be4 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -378,8 +378,8 @@ EVP_tp_init(EVPobject *self, PyObject *args, PyObject *kwds) return -1; } - self->name = name_obj; - Py_INCREF(self->name); + Py_INCREF(name_obj); + Py_XSETREF(self->name, name_obj); if (data_obj) { if (view.len >= HASHLIB_GIL_MINSIZE) { diff --git a/Modules/_lzmamodule.c b/Modules/_lzmamodule.c index bb77552b67b15d..b25faff5ac4924 100644 --- a/Modules/_lzmamodule.c +++ b/Modules/_lzmamodule.c @@ -1192,7 +1192,7 @@ _lzma_LZMADecompressor___init___impl(Decompressor *self, int format, self->needs_input = 1; self->input_buffer = NULL; self->input_buffer_size = 0; - self->unused_data = PyBytes_FromStringAndSize(NULL, 0); + Py_XSETREF(self->unused_data, PyBytes_FromStringAndSize(NULL, 0)); if (self->unused_data == NULL) goto error; diff --git a/Objects/descrobject.c b/Objects/descrobject.c index 076e7414814825..ef91aabaf63174 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1483,11 +1483,11 @@ property_init(PyObject *self, PyObject *args, PyObject *kwds) Py_XINCREF(del); Py_XINCREF(doc); - prop->prop_get = get; - prop->prop_set = set; - prop->prop_del = del; - prop->prop_doc = doc; - prop->getter_doc = 0; + Py_XSETREF(prop->prop_get, fget); + Py_XSETREF(prop->prop_set, fset); + Py_XSETREF(prop->prop_del, fdel); + Py_XSETREF(prop->prop_doc, doc); + self->getter_doc = 0; /* if no docstring given and the getter has one, use that one */ if ((doc == NULL || doc == Py_None) && get != NULL) { diff --git a/Objects/funcobject.c b/Objects/funcobject.c index 69cd973384b204..6ce0cb43e5e929 100644 --- a/Objects/funcobject.c +++ b/Objects/funcobject.c @@ -745,7 +745,7 @@ cm_init(PyObject *self, PyObject *args, PyObject *kwds) if (!_PyArg_NoKeywords("classmethod", kwds)) return -1; Py_INCREF(callable); - cm->cm_callable = callable; + Py_XSETREF(cm->cm_callable, callable); return 0; } @@ -926,7 +926,7 @@ sm_init(PyObject *self, PyObject *args, PyObject *kwds) if (!_PyArg_NoKeywords("staticmethod", kwds)) return -1; Py_INCREF(callable); - sm->sm_callable = callable; + Py_XSETREF(sm->sm_callable, callable); return 0; } From cab440fce6dc31593e1e875315b96067fead396d Mon Sep 17 00:00:00 2001 From: INADA Naoki Date: Tue, 13 Feb 2018 20:48:24 +0900 Subject: [PATCH 2/2] fix --- Modules/_asynciomodule.c | 2 -- Objects/descrobject.c | 8 ++++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 0d887ba802ca6d..f53387115e73aa 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -142,8 +142,6 @@ future_init(FutureObj *fut, PyObject *loop) // Same to FutureObj_clear() but not clearing fut->dict Py_CLEAR(fut->fut_loop); - Py_CLEAR(fut->fut_callback0); - Py_CLEAR(fut->fut_context0); Py_CLEAR(fut->fut_callbacks); Py_CLEAR(fut->fut_result); Py_CLEAR(fut->fut_exception); diff --git a/Objects/descrobject.c b/Objects/descrobject.c index ef91aabaf63174..9020ccd3254792 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1483,11 +1483,11 @@ property_init(PyObject *self, PyObject *args, PyObject *kwds) Py_XINCREF(del); Py_XINCREF(doc); - Py_XSETREF(prop->prop_get, fget); - Py_XSETREF(prop->prop_set, fset); - Py_XSETREF(prop->prop_del, fdel); + Py_XSETREF(prop->prop_get, get); + Py_XSETREF(prop->prop_set, set); + Py_XSETREF(prop->prop_del, del); Py_XSETREF(prop->prop_doc, doc); - self->getter_doc = 0; + prop->getter_doc = 0; /* if no docstring given and the getter has one, use that one */ if ((doc == NULL || doc == Py_None) && get != NULL) {