From 01edb37b431d2142bae15516d8424c7277fa9005 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 12 May 2021 17:48:58 +0200 Subject: [PATCH 1/4] bpo-44116: Add GC support to _csv heap types --- Modules/_csv.c | 43 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/Modules/_csv.c b/Modules/_csv.c index bdb67fdb4876fe..bed5f70e9969dc 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -315,6 +315,7 @@ static PyGetSetDef Dialect_getsetlist[] = { static void Dialect_dealloc(DialectObj *self) { + PyObject_GC_UnTrack(self); PyTypeObject *tp = Py_TYPE(self); Py_CLEAR(self->lineterminator); tp->tp_free((PyObject *)self); @@ -512,6 +513,13 @@ PyDoc_STRVAR(Dialect_Type_doc, "\n" "The Dialect type records CSV parsing and generation options.\n"); +static int +Dialect_traverse(PyObject *self, visitproc visit, void *arg) +{ + Py_VISIT(Py_TYPE(self)); + return 0; +} + static PyType_Slot Dialect_Type_slots[] = { {Py_tp_doc, (char*)Dialect_Type_doc}, {Py_tp_members, Dialect_memberlist}, @@ -520,13 +528,14 @@ static PyType_Slot Dialect_Type_slots[] = { {Py_tp_methods, dialect_methods}, {Py_tp_finalize, Dialect_finalize}, {Py_tp_dealloc, Dialect_dealloc}, + {Py_tp_traverse, Dialect_traverse}, {0, NULL} }; PyType_Spec Dialect_Type_spec = { .name = "_csv.Dialect", .basicsize = sizeof(DialectObj), - .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, + .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, .slots = Dialect_Type_slots, }; @@ -914,6 +923,7 @@ Reader_traverse(ReaderObj *self, visitproc visit, void *arg) Py_VISIT(self->dialect); Py_VISIT(self->input_iter); Py_VISIT(self->fields); + Py_VISIT(Py_TYPE(self)); return 0; } @@ -1339,6 +1349,7 @@ Writer_traverse(WriterObj *self, visitproc visit, void *arg) Py_VISIT(self->dialect); Py_VISIT(self->write); Py_VISIT(self->error_obj); + Py_VISIT(Py_TYPE(self)); return 0; } @@ -1351,6 +1362,15 @@ Writer_clear(WriterObj *self) return 0; } +static void +Writer_dealloc(WriterObj *self) +{ + PyObject_GC_UnTrack(self); + PyTypeObject *tp = Py_TYPE(self); + tp->tp_clear((PyObject *)self); + Py_DECREF(tp); +} + static void Writer_finalize(WriterObj *self) { @@ -1372,6 +1392,7 @@ static PyType_Slot Writer_Type_slots[] = { {Py_tp_doc, (char*)Writer_Type_doc}, {Py_tp_traverse, Writer_traverse}, {Py_tp_clear, Writer_clear}, + {Py_tp_dealloc, Writer_dealloc}, {Py_tp_methods, Writer_methods}, {Py_tp_members, Writer_memberlist}, {0, NULL} @@ -1509,13 +1530,31 @@ csv_field_size_limit(PyObject *module, PyObject *args) return PyLong_FromLong(old_limit); } +static void +error_dealloc(PyObject *self) +{ + PyObject_GC_UnTrack(self); + PyTypeObject *tp = Py_TYPE(self); + tp->tp_free((PyObject *)self); + Py_DECREF(tp); +} + +static int +error_traverse(PyObject *self, visitproc visit, void *arg) +{ + Py_VISIT(Py_TYPE(self)); + return 0; +} + static PyType_Slot error_slots[] = { + {Py_tp_traverse, error_traverse}, + {Py_tp_dealloc, error_dealloc}, {0, NULL}, }; PyType_Spec error_spec = { .name = "_csv.Error", - .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, + .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, .slots = error_slots, }; From b8772d6d405c4160d8ce19f58074c92e1d4ab7c0 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 12 May 2021 18:22:00 +0200 Subject: [PATCH 2/4] Revert _csv.Error changes --- Modules/_csv.c | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/Modules/_csv.c b/Modules/_csv.c index bed5f70e9969dc..dd9dcce620e242 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -1530,31 +1530,13 @@ csv_field_size_limit(PyObject *module, PyObject *args) return PyLong_FromLong(old_limit); } -static void -error_dealloc(PyObject *self) -{ - PyObject_GC_UnTrack(self); - PyTypeObject *tp = Py_TYPE(self); - tp->tp_free((PyObject *)self); - Py_DECREF(tp); -} - -static int -error_traverse(PyObject *self, visitproc visit, void *arg) -{ - Py_VISIT(Py_TYPE(self)); - return 0; -} - static PyType_Slot error_slots[] = { - {Py_tp_traverse, error_traverse}, - {Py_tp_dealloc, error_dealloc}, {0, NULL}, }; PyType_Spec error_spec = { .name = "_csv.Error", - .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, + .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, .slots = error_slots, }; From 19e2952e43e013f1d49fd0437878e4b1c7f0ce66 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 12 May 2021 18:51:45 +0200 Subject: [PATCH 3/4] Use tp_clear iso. tp_finalize --- Modules/_csv.c | 67 ++++++++++++++++++-------------------------------- 1 file changed, 24 insertions(+), 43 deletions(-) diff --git a/Modules/_csv.c b/Modules/_csv.c index dd9dcce620e242..cc3c9c8a74608d 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -315,19 +315,13 @@ static PyGetSetDef Dialect_getsetlist[] = { static void Dialect_dealloc(DialectObj *self) { - PyObject_GC_UnTrack(self); PyTypeObject *tp = Py_TYPE(self); - Py_CLEAR(self->lineterminator); - tp->tp_free((PyObject *)self); + PyObject_GC_UnTrack(self); + tp->tp_clear((PyObject *)self); + PyObject_GC_Del(self); Py_DECREF(tp); } -static void -Dialect_finalize(DialectObj *self) -{ - Py_CLEAR(self->lineterminator); -} - static char *dialect_kws[] = { "dialect", "delimiter", @@ -514,8 +508,16 @@ PyDoc_STRVAR(Dialect_Type_doc, "The Dialect type records CSV parsing and generation options.\n"); static int -Dialect_traverse(PyObject *self, visitproc visit, void *arg) +Dialect_clear(DialectObj *self) { + Py_CLEAR(self->lineterminator); + return 0; +} + +static int +Dialect_traverse(DialectObj *self, visitproc visit, void *arg) +{ + Py_VISIT(self->lineterminator); Py_VISIT(Py_TYPE(self)); return 0; } @@ -526,8 +528,8 @@ static PyType_Slot Dialect_Type_slots[] = { {Py_tp_getset, Dialect_getsetlist}, {Py_tp_new, dialect_new}, {Py_tp_methods, dialect_methods}, - {Py_tp_finalize, Dialect_finalize}, {Py_tp_dealloc, Dialect_dealloc}, + {Py_tp_clear, Dialect_clear}, {Py_tp_traverse, Dialect_traverse}, {0, NULL} }; @@ -894,29 +896,11 @@ Reader_dealloc(ReaderObj *self) { PyTypeObject *tp = Py_TYPE(self); PyObject_GC_UnTrack(self); - Py_CLEAR(self->dialect); - Py_CLEAR(self->input_iter); - Py_CLEAR(self->fields); - if (self->field != NULL) { - PyMem_Free(self->field); - self->field = NULL; - } + tp->tp_clear((PyObject *)self); PyObject_GC_Del(self); Py_DECREF(tp); } -static void -Reader_finalize(ReaderObj *self) -{ - Py_CLEAR(self->dialect); - Py_CLEAR(self->input_iter); - Py_CLEAR(self->fields); - if (self->field != NULL) { - PyMem_Free(self->field); - self->field = NULL; - } -} - static int Reader_traverse(ReaderObj *self, visitproc visit, void *arg) { @@ -933,6 +917,10 @@ Reader_clear(ReaderObj *self) Py_CLEAR(self->dialect); Py_CLEAR(self->input_iter); Py_CLEAR(self->fields); + if (self->field != NULL) { + PyMem_Free(self->field); + self->field = NULL; + } return 0; } @@ -958,12 +946,11 @@ static struct PyMemberDef Reader_memberlist[] = { static PyType_Slot Reader_Type_slots[] = { {Py_tp_doc, (char*)Reader_Type_doc}, {Py_tp_traverse, Reader_traverse}, - {Py_tp_clear, Reader_clear}, {Py_tp_iter, PyObject_SelfIter}, {Py_tp_iternext, Reader_iternext}, {Py_tp_methods, Reader_methods}, {Py_tp_members, Reader_memberlist}, - {Py_tp_finalize, Reader_finalize}, + {Py_tp_clear, Reader_clear}, {Py_tp_dealloc, Reader_dealloc}, {0, NULL} }; @@ -1359,27 +1346,22 @@ Writer_clear(WriterObj *self) Py_CLEAR(self->dialect); Py_CLEAR(self->write); Py_CLEAR(self->error_obj); + if (self->rec != NULL) { + PyMem_Free(self->rec); + } return 0; } static void Writer_dealloc(WriterObj *self) { - PyObject_GC_UnTrack(self); PyTypeObject *tp = Py_TYPE(self); + PyObject_GC_UnTrack(self); tp->tp_clear((PyObject *)self); + PyObject_GC_Del(self); Py_DECREF(tp); } -static void -Writer_finalize(WriterObj *self) -{ - Writer_clear(self); - if (self->rec != NULL) { - PyMem_Free(self->rec); - } -} - PyDoc_STRVAR(Writer_Type_doc, "CSV writer\n" "\n" @@ -1388,7 +1370,6 @@ PyDoc_STRVAR(Writer_Type_doc, ); static PyType_Slot Writer_Type_slots[] = { - {Py_tp_finalize, Writer_finalize}, {Py_tp_doc, (char*)Writer_Type_doc}, {Py_tp_traverse, Writer_traverse}, {Py_tp_clear, Writer_clear}, From fe74fc2dd9ca1a59e3f12a0bb5adaf0c36834b85 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 12 May 2021 19:07:59 +0200 Subject: [PATCH 4/4] Only dealloc should free memory --- Modules/_csv.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Modules/_csv.c b/Modules/_csv.c index cc3c9c8a74608d..a213734f508068 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -897,6 +897,10 @@ Reader_dealloc(ReaderObj *self) PyTypeObject *tp = Py_TYPE(self); PyObject_GC_UnTrack(self); tp->tp_clear((PyObject *)self); + if (self->field != NULL) { + PyMem_Free(self->field); + self->field = NULL; + } PyObject_GC_Del(self); Py_DECREF(tp); } @@ -917,10 +921,6 @@ Reader_clear(ReaderObj *self) Py_CLEAR(self->dialect); Py_CLEAR(self->input_iter); Py_CLEAR(self->fields); - if (self->field != NULL) { - PyMem_Free(self->field); - self->field = NULL; - } return 0; } @@ -1346,9 +1346,6 @@ Writer_clear(WriterObj *self) Py_CLEAR(self->dialect); Py_CLEAR(self->write); Py_CLEAR(self->error_obj); - if (self->rec != NULL) { - PyMem_Free(self->rec); - } return 0; } @@ -1358,6 +1355,9 @@ Writer_dealloc(WriterObj *self) PyTypeObject *tp = Py_TYPE(self); PyObject_GC_UnTrack(self); tp->tp_clear((PyObject *)self); + if (self->rec != NULL) { + PyMem_Free(self->rec); + } PyObject_GC_Del(self); Py_DECREF(tp); }