From ed3cd983f77707ddbb3cc5df8041e4bb1068c9cb Mon Sep 17 00:00:00 2001 From: Hai Shi Date: Tue, 23 Jun 2020 21:12:11 +0800 Subject: [PATCH 1/6] Enable unicode_release_interned() without insure or valgrind. --- Objects/unicodeobject.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index 1433848c81f8e1..0968ab9905b8c0 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -15657,7 +15657,6 @@ PyUnicode_InternFromString(const char *cp) } -#if defined(WITH_VALGRIND) || defined(__INSURE__) static void unicode_release_interned(void) { @@ -15715,7 +15714,6 @@ unicode_release_interned(void) PyDict_Clear(interned); Py_CLEAR(interned); } -#endif /********************* Unicode Iterator **************************/ @@ -16206,18 +16204,7 @@ void _PyUnicode_Fini(PyThreadState *tstate) { if (_Py_IsMainInterpreter(tstate)) { -#if defined(WITH_VALGRIND) || defined(__INSURE__) - /* Insure++ is a memory analysis tool that aids in discovering - * memory leaks and other memory problems. On Python exit, the - * interned string dictionaries are flagged as being in use at exit - * (which it is). Under normal circumstances, this is fine because - * the memory will be automatically reclaimed by the system. Under - * memory debugging, it's a huge source of useless noise, so we - * trade off slower shutdown for less distraction in the memory - * reports. -baw - */ unicode_release_interned(); -#endif /* __INSURE__ */ Py_CLEAR(unicode_empty); From 3c8db5036b6b37b8be2a3e3b319e2d510d940441 Mon Sep 17 00:00:00 2001 From: Hai Shi Date: Tue, 23 Jun 2020 23:52:53 +0800 Subject: [PATCH 2/6] resolve use after free --- Python/pylifecycle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 87f25e623f570d..cde676b3a48a6c 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1261,7 +1261,6 @@ finalize_interp_types(PyThreadState *tstate, int is_main_interp) if (is_main_interp) { _PyDict_Fini(); } - _PyList_Fini(tstate); _PyTuple_Fini(tstate); _PySlice_Fini(tstate); @@ -1270,6 +1269,7 @@ finalize_interp_types(PyThreadState *tstate, int is_main_interp) _PyBytes_Fini(); } _PyUnicode_Fini(tstate); + _PyList_Fini(tstate); _PyFloat_Fini(tstate); _PyLong_Fini(tstate); } From b2e578dc4f78a95ec289f6ded812c6c02b3e9df7 Mon Sep 17 00:00:00 2001 From: Hai Shi Date: Wed, 24 Jun 2020 12:43:27 +0800 Subject: [PATCH 3/6] resolve confilicts --- Objects/unicodeobject.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index 5ba99514d29691..65d353cf78e7d3 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -15657,7 +15657,6 @@ PyUnicode_InternFromString(const char *cp) } -#if defined(WITH_VALGRIND) || defined(__INSURE__) static void unicode_release_interned(void) { @@ -15715,7 +15714,6 @@ unicode_release_interned(void) PyDict_Clear(interned); Py_CLEAR(interned); } -#endif /********************* Unicode Iterator **************************/ @@ -16209,18 +16207,7 @@ _PyUnicode_Fini(PyThreadState *tstate) int is_main_interp = _Py_IsMainInterpreter(tstate); if (is_main_interp) { -#if defined(WITH_VALGRIND) || defined(__INSURE__) - /* Insure++ is a memory analysis tool that aids in discovering - * memory leaks and other memory problems. On Python exit, the - * interned string dictionaries are flagged as being in use at exit - * (which it is). Under normal circumstances, this is fine because - * the memory will be automatically reclaimed by the system. Under - * memory debugging, it's a huge source of useless noise, so we - * trade off slower shutdown for less distraction in the memory - * reports. -baw - */ unicode_release_interned(); -#endif /* __INSURE__ */ } Py_CLEAR(state->empty); From 4618783efbbcc3091e0069db0e271ec2deddf73b Mon Sep 17 00:00:00 2001 From: Hai Shi Date: Thu, 25 Jun 2020 15:49:50 +0800 Subject: [PATCH 4/6] Use PyDict_Next() in unicode_release_interned() --- Objects/unicodeobject.c | 46 ++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index 65d353cf78e7d3..b9c25a71b381b0 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -15660,57 +15660,51 @@ PyUnicode_InternFromString(const char *cp) static void unicode_release_interned(void) { + Py_ssize_t pos = 0; + PyObject *key, *value; + if (interned == NULL || !PyDict_Check(interned)) { return; } - PyObject *keys = PyDict_Keys(interned); - if (keys == NULL || !PyList_Check(keys)) { - PyErr_Clear(); - return; - } /* Since unicode_release_interned() is intended to help a leak detector, interned unicode strings are not forcibly deallocated; rather, we give them their stolen references back, and then clear and DECREF the interned dict. */ - - Py_ssize_t n = PyList_GET_SIZE(keys); #ifdef INTERNED_STATS - fprintf(stderr, "releasing %zd interned strings\n", n); + fprintf(stderr, "releasing %zd interned strings\n", PyDict_Size(interned)); Py_ssize_t immortal_size = 0, mortal_size = 0; #endif - for (Py_ssize_t i = 0; i < n; i++) { - PyObject *s = PyList_GET_ITEM(keys, i); - if (PyUnicode_READY(s) == -1) { + while (PyDict_Next(interned, &pos, &key, &value)) { + if (PyUnicode_READY(key) == -1) { Py_UNREACHABLE(); } - switch (PyUnicode_CHECK_INTERNED(s)) { - case SSTATE_INTERNED_IMMORTAL: - Py_SET_REFCNT(s, Py_REFCNT(s) + 1); + switch (PyUnicode_CHECK_INTERNED(key)) { + case SSTATE_INTERNED_IMMORTAL: + Py_SET_REFCNT(key, Py_REFCNT(key) + 1); #ifdef INTERNED_STATS - immortal_size += PyUnicode_GET_LENGTH(s); + immortal_size += PyUnicode_GET_LENGTH(key); #endif - break; - case SSTATE_INTERNED_MORTAL: - Py_SET_REFCNT(s, Py_REFCNT(s) + 2); + break; + case SSTATE_INTERNED_MORTAL: + Py_SET_REFCNT(key, Py_REFCNT(key) + 2); #ifdef INTERNED_STATS - mortal_size += PyUnicode_GET_LENGTH(s); + mortal_size += PyUnicode_GET_LENGTH(key); #endif - break; - case SSTATE_NOT_INTERNED: - /* fall through */ - default: - Py_UNREACHABLE(); + break; + case SSTATE_NOT_INTERNED: + /* fall through */ + default: + Py_UNREACHABLE(); } - _PyUnicode_STATE(s).interned = SSTATE_NOT_INTERNED; + _PyUnicode_STATE(key).interned = SSTATE_NOT_INTERNED; } #ifdef INTERNED_STATS fprintf(stderr, "total size of all interned strings: %zd/%zd mortal/immortal\n", mortal_size, immortal_size); #endif - Py_DECREF(keys); PyDict_Clear(interned); Py_CLEAR(interned); } From ec1671b8c7f07e9a30d9884e5e66803e4bf8d42d Mon Sep 17 00:00:00 2001 From: Hai Shi Date: Thu, 25 Jun 2020 17:36:01 +0800 Subject: [PATCH 5/6] update vars' name --- Objects/unicodeobject.c | 18 +++++++++--------- Python/pylifecycle.c | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index b9c25a71b381b0..a5b6a3d873ba1d 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -15661,7 +15661,7 @@ static void unicode_release_interned(void) { Py_ssize_t pos = 0; - PyObject *key, *value; + PyObject *s, *ignored_value; if (interned == NULL || !PyDict_Check(interned)) { return; @@ -15676,21 +15676,21 @@ unicode_release_interned(void) Py_ssize_t immortal_size = 0, mortal_size = 0; #endif - while (PyDict_Next(interned, &pos, &key, &value)) { - if (PyUnicode_READY(key) == -1) { + while (PyDict_Next(interned, &pos, &s, &ignored_value)) { + if (PyUnicode_READY(s) == -1) { Py_UNREACHABLE(); } - switch (PyUnicode_CHECK_INTERNED(key)) { + switch (PyUnicode_CHECK_INTERNED(s)) { case SSTATE_INTERNED_IMMORTAL: - Py_SET_REFCNT(key, Py_REFCNT(key) + 1); + Py_SET_REFCNT(s, Py_REFCNT(s) + 1); #ifdef INTERNED_STATS - immortal_size += PyUnicode_GET_LENGTH(key); + immortal_size += PyUnicode_GET_LENGTH(s); #endif break; case SSTATE_INTERNED_MORTAL: - Py_SET_REFCNT(key, Py_REFCNT(key) + 2); + Py_SET_REFCNT(s, Py_REFCNT(s) + 2); #ifdef INTERNED_STATS - mortal_size += PyUnicode_GET_LENGTH(key); + mortal_size += PyUnicode_GET_LENGTH(s); #endif break; case SSTATE_NOT_INTERNED: @@ -15698,7 +15698,7 @@ unicode_release_interned(void) default: Py_UNREACHABLE(); } - _PyUnicode_STATE(key).interned = SSTATE_NOT_INTERNED; + _PyUnicode_STATE(s).interned = SSTATE_NOT_INTERNED; } #ifdef INTERNED_STATS fprintf(stderr, diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index ec81e197b19cfd..4b658f847bc12b 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1260,13 +1260,13 @@ finalize_interp_types(PyThreadState *tstate, int is_main_interp) _PyContext_Fini(tstate); _PyDict_Fini(tstate); + _PyList_Fini(tstate); _PyTuple_Fini(tstate); _PySlice_Fini(tstate); _PyBytes_Fini(tstate); _PyUnicode_Fini(tstate); - _PyList_Fini(tstate); _PyFloat_Fini(tstate); _PyLong_Fini(tstate); } From b0da3bd49df9085108d3013315742928d4f85fad Mon Sep 17 00:00:00 2001 From: Hai Shi Date: Thu, 2 Jul 2020 00:53:31 +0800 Subject: [PATCH 6/6] _PyUnicode_Fini() before _PyDict_Fini() --- Python/pylifecycle.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 4b658f847bc12b..b0e6005cce2254 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1259,14 +1259,14 @@ finalize_interp_types(PyThreadState *tstate, int is_main_interp) _PyAsyncGen_Fini(tstate); _PyContext_Fini(tstate); - _PyDict_Fini(tstate); _PyList_Fini(tstate); + _PyUnicode_Fini(tstate); + _PyDict_Fini(tstate); _PyTuple_Fini(tstate); _PySlice_Fini(tstate); _PyBytes_Fini(tstate); - _PyUnicode_Fini(tstate); _PyFloat_Fini(tstate); _PyLong_Fini(tstate); }