From fe5b73f61579f8bf945dab91762311384d9d99cf Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 23 Aug 2024 06:08:59 +0900 Subject: [PATCH 1/8] add GC Track --- Modules/_decimal/_decimal.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index 15855e2cf1bb32..94398bed6b100c 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -1390,6 +1390,9 @@ context_new(PyTypeObject *type, PyObject *args UNUSED, PyObject *kwds UNUSED) CtxCaps(self) = 1; self->tstate = NULL; + if (type == state->PyDecContext_Type) { + PyObject_GC_Track(self); + } return (PyObject *)self; } @@ -2038,6 +2041,9 @@ PyDecType_New(PyTypeObject *type) MPD(dec)->alloc = _Py_DEC_MINALLOC; MPD(dec)->data = dec->data; + if (type == state->PyDec_Type) { + PyObject_GC_Track(dec); + } return (PyObject *)dec; } #define dec_alloc(st) PyDecType_New((st)->PyDec_Type) From bfc51af38d830f6cb4820e9a7bc3c22bbb962b0d Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 23 Aug 2024 06:10:18 +0900 Subject: [PATCH 2/8] fix double free --- Modules/_decimal/_decimal.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index 94398bed6b100c..65bbd6a90fe9bb 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -6151,6 +6151,8 @@ decimal_clear(PyObject *module) PyMem_Free(state->signal_map); PyMem_Free(state->cond_map); + state->signal_map = NULL; + state->cond_map = NULL; return 0; } From ee82db6c526b34369623da91ed064aec1bfe2165 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 23 Aug 2024 06:10:50 +0900 Subject: [PATCH 3/8] fix exception leak --- Modules/_decimal/_decimal.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index 65bbd6a90fe9bb..577f33a8c3c8ac 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -6149,10 +6149,22 @@ decimal_clear(PyObject *module) Py_CLEAR(state->SignalTuple); Py_CLEAR(state->PyDecimal); - PyMem_Free(state->signal_map); - PyMem_Free(state->cond_map); - state->signal_map = NULL; - state->cond_map = NULL; + if (state->signal_map != NULL) { + for (DecCondMap *cm = state->signal_map; cm->name != NULL; cm++) { + Py_CLEAR(cm->ex); + } + PyMem_Free(state->signal_map); + state->signal_map = NULL; + } + + if (state->cond_map != NULL) { + // signal_map[0] is assigned to cond_map[0] + for (DecCondMap *cm = state->cond_map + 1; cm->name != NULL; cm++) { + Py_CLEAR(cm->ex); + } + PyMem_Free(state->cond_map); + state->cond_map = NULL; + } return 0; } From d1373d9759cab7e11e1dfc7d8a0781969f766151 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 23 Aug 2024 06:11:57 +0900 Subject: [PATCH 4/8] NEWS --- .../next/Library/2024-08-22-20-10-13.gh-issue-123243.Kifj1L.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2024-08-22-20-10-13.gh-issue-123243.Kifj1L.rst diff --git a/Misc/NEWS.d/next/Library/2024-08-22-20-10-13.gh-issue-123243.Kifj1L.rst b/Misc/NEWS.d/next/Library/2024-08-22-20-10-13.gh-issue-123243.Kifj1L.rst new file mode 100644 index 00000000000000..cf52585020111f --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-08-22-20-10-13.gh-issue-123243.Kifj1L.rst @@ -0,0 +1 @@ +Fix memory leak in :mod:`!_decimal`. From 20b668ced0a4146098dfc5e8bab0f6f7f4aa8989 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 23 Aug 2024 11:07:54 +0900 Subject: [PATCH 5/8] make code more explicit --- Modules/_decimal/_decimal.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index 577f33a8c3c8ac..9d224dca0e0419 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -1357,6 +1357,7 @@ context_new(PyTypeObject *type, PyObject *args UNUSED, PyObject *kwds UNUSED) } else { self = (PyDecContextObject *)type->tp_alloc(type, 0); + assert(PyObject_GC_IsTracked(self)); } if (self == NULL) { @@ -2027,6 +2028,7 @@ PyDecType_New(PyTypeObject *type) } else { dec = (PyDecObject *)type->tp_alloc(type, 0); + assert(PyObject_GC_IsTracked(dec)); } if (dec == NULL) { return NULL; @@ -6158,7 +6160,7 @@ decimal_clear(PyObject *module) } if (state->cond_map != NULL) { - // signal_map[0] is assigned to cond_map[0] + state->cond_map[0].ex = NULL; // decref'ed at signal_map[0].ex for (DecCondMap *cm = state->cond_map + 1; cm->name != NULL; cm++) { Py_CLEAR(cm->ex); } From b6e4279486dd7b8c291e1248f9d091f7174678e2 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 23 Aug 2024 11:43:36 +0900 Subject: [PATCH 6/8] fix warning --- Modules/_decimal/_decimal.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index 9d224dca0e0419..78b03488dc3cef 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -1357,7 +1357,7 @@ context_new(PyTypeObject *type, PyObject *args UNUSED, PyObject *kwds UNUSED) } else { self = (PyDecContextObject *)type->tp_alloc(type, 0); - assert(PyObject_GC_IsTracked(self)); + assert(PyObject_GC_IsTracked((PyObject *)self)); } if (self == NULL) { @@ -2028,7 +2028,7 @@ PyDecType_New(PyTypeObject *type) } else { dec = (PyDecObject *)type->tp_alloc(type, 0); - assert(PyObject_GC_IsTracked(dec)); + assert(PyObject_GC_IsTracked((PyObject *)dec)); } if (dec == NULL) { return NULL; From f8d1a2d2487cc861d714080b52db89c96704d136 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 23 Aug 2024 16:37:40 +0900 Subject: [PATCH 7/8] use Py_DECREF() rather than Py_CLEAR() --- Modules/_decimal/_decimal.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index 78b03488dc3cef..a234d94cf2ce40 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -6153,16 +6153,16 @@ decimal_clear(PyObject *module) if (state->signal_map != NULL) { for (DecCondMap *cm = state->signal_map; cm->name != NULL; cm++) { - Py_CLEAR(cm->ex); + Py_DECREF(cm->ex); } PyMem_Free(state->signal_map); state->signal_map = NULL; } if (state->cond_map != NULL) { - state->cond_map[0].ex = NULL; // decref'ed at signal_map[0].ex + // cond_map[0].ex has borrowed a reference from signal_map[0].ex for (DecCondMap *cm = state->cond_map + 1; cm->name != NULL; cm++) { - Py_CLEAR(cm->ex); + Py_DECREF(cm->ex); } PyMem_Free(state->cond_map); state->cond_map = NULL; From 33af89b8f0dbb137657cfb887407f62771141efd Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 23 Aug 2024 18:02:49 +0900 Subject: [PATCH 8/8] ensure GC is tracked on return --- Modules/_decimal/_decimal.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index a234d94cf2ce40..0e743a609d33d6 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -1357,7 +1357,6 @@ context_new(PyTypeObject *type, PyObject *args UNUSED, PyObject *kwds UNUSED) } else { self = (PyDecContextObject *)type->tp_alloc(type, 0); - assert(PyObject_GC_IsTracked((PyObject *)self)); } if (self == NULL) { @@ -1394,6 +1393,7 @@ context_new(PyTypeObject *type, PyObject *args UNUSED, PyObject *kwds UNUSED) if (type == state->PyDecContext_Type) { PyObject_GC_Track(self); } + assert(PyObject_GC_IsTracked((PyObject *)self)); return (PyObject *)self; } @@ -2028,7 +2028,6 @@ PyDecType_New(PyTypeObject *type) } else { dec = (PyDecObject *)type->tp_alloc(type, 0); - assert(PyObject_GC_IsTracked((PyObject *)dec)); } if (dec == NULL) { return NULL; @@ -2046,6 +2045,7 @@ PyDecType_New(PyTypeObject *type) if (type == state->PyDec_Type) { PyObject_GC_Track(dec); } + assert(PyObject_GC_IsTracked((PyObject *)dec)); return (PyObject *)dec; } #define dec_alloc(st) PyDecType_New((st)->PyDec_Type)