| msg192369 - (view) |
Author: Christian Heimes (christian.heimes) *  |
Date: 2013-07-05 22:53 |
| In _Pickler_New() PicklerObject is allocated with self = PyObject_GC_New(PicklerObject, &Pickler_Type); but PyObject_GC_Track(self) is never called, same for _Unpickler_New(). I think it's a bug. |
|
|
| msg228604 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2014-10-05 18:04 |
| @Christian/Victor could either of you provide a patch for this, it's way beyond my knowledge I'm afraid. |
|
|
| msg228610 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2014-10-05 18:25 |
| Mark, could you please stop touching every issue on the tracker? I appreciate the effort, but giving thoughtful feedback, patches, or reviews on a just a few issues would be much more helpful. |
|
|
| msg228613 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2014-10-05 18:40 |
| Sorry but no, when I started out on this a couple of months ago there were over 600 issues that nobody had even bothered to reply to. That number is now down to 369. I believe that around 200 issues have been closed as a result of my efforts. Do you have a problem with me showing just how easy it is to get issues resolved? |
|
|
| msg228617 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2014-10-05 19:35 |
| Mark, I also appreciate your efforts, as I hope should be obvious from my prior responses to many of your other posts. I also agree that somewhat fewer, higher quality posts would be even more helpful. Please do not slip into a 'confrontational' mode, as you did a few years ago. And please lets discuss this further by private email. |
|
|
| msg228619 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2014-10-05 21:20 |
| Le 5 oct. 2014 20:04, "Mark Lawrence" <report@bugs.python.org> a écrit : > > > Mark Lawrence added the comment: > > @Christian/Victor could either of you provide a patch for this, it's way beyond my knowledge I'm afraid. I agree with Benjamin. Asking directly two developers to write a patch don't add any value to the issue, it only puts pressure on them. If I didn't write a patch before, it's probably because I'm not really interested by the topic. If you would like to contribute, you can close directly issues, test and review patches, suggest an implementation, etc. It's not all black or all white. Sometimes, your "ping" messages are helpful reminders to finish the work or simply to close the issue. By experience, most old issues are not finished because the bug only impact a few people and it's possible to work around it, or because the feature request is not interesting enough. I didn't read this issue (i'm replying in my mail client), my remarks are general. |
|
|
| msg228621 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2014-10-05 21:24 |
| I don't know the PyObject_GC_Track() function. Why is it an issue to not call it? Can you elaborate Christian? What do you suggest? |
|
|
| msg228622 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2014-10-05 21:52 |
| From Doc/c-api/gcsupport.rst: > Constructors for container types must conform to two rules: > > #. The memory for the object must be allocated using :c:func:`PyObject_GC_New` > or :c:func:`PyObject_GC_NewVar`. > > #. Once all the fields which may contain references to other containers are > > initialized, it must call :c:func:`PyObject_GC_Track`. _pickle.Pickler and _pickle.Unpickler have the Py_TPFLAGS_HAVE_GC flag, implement tp_traverse and tp_clear, but PyObject_GC_Track is newer called. |
|
|
| msg339604 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2019-04-08 10:21 |
| It has been decided to not fix the issue in Python 2.7: https://github.com/python/cpython/pull/8505#issuecomment-480771689 |
|
|
| msg339642 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2019-04-08 14:19 |
| Tracking objects that do not need this will just add work to the garbage collector. Not all instances of trackable types should be tracked, for example the empty tuple and some dicts are not tracked. >>> gc.is_tracked(()) False >>> gc.is_tracked((1, 2)) True >>> gc.is_tracked({1: None}) False >>> gc.is_tracked({1: []}) True |
|
|
| msg339724 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2019-04-09 10:36 |
| > Tracking objects that do not need this will just add work to the garbage collector. Not all instances of trackable types should be tracked, for example the empty tuple and some dicts are not tracked. Well, in that case, we should do the opposite of PR 8505, what I proposed there: https://github.com/python/cpython/pull/8505#issuecomment-480763122 "Either GC support must be removed (remove Py_TPFLAGS_HAVE_GC, remove tp_clear and tp_traverse, etc.), or the implementation should be fixed (call PyObject_GC_Track)." => fully remove the GC support I don't see the point of implementing tp_traverse if it's not called. I'm not sure if tp_clear is related to the GC or not. Maybe keep it :-) |
|
|
| msg339726 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2019-04-09 10:45 |
| GC support can not be removed, because these objects are created also as long living objects using the standard way. And in this case they are tracked (in PyType_GenericAlloc()) and tp_traverse is called. They are not tracked only when created by an internal API _Pickler_New() and _Unpickler_New(). The current code is correct and I do not see reasons to change it. |
|
|
| msg340718 - (view) |
Author: Inada Naoki (methane) * |
Date: 2019-04-23 11:56 |
| New changeset 359bd4f61b9e1493081f4f67882554247b53926a by Inada Naoki (Zackery Spytz) in branch 'master': bpo-18372: Add missing PyObject_GC_Track() calls in the pickle module (GH-8505) https://github.com/python/cpython/commit/359bd4f61b9e1493081f4f67882554247b53926a |
|
|
| msg340719 - (view) |
Author: miss-islington (miss-islington) |
Date: 2019-04-23 12:18 |
| New changeset c0f6f5370325459cadd90010530b1d300dce514e by Miss Islington (bot) in branch '3.7': bpo-18372: Add missing PyObject_GC_Track() calls in the pickle module (GH-8505) https://github.com/python/cpython/commit/c0f6f5370325459cadd90010530b1d300dce514e |
|
|