This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: _tkinter should use Python PyMem_Malloc() instead of Tcl ckalloc()
Type: enhancement Stage: resolved
Components: Tkinter Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: David.Edelsohn, python-dev, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2014-09-04 15:46 by vstinner, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
pymem.patch vstinner, 2014-09-04 15:46 review
tkinter_pymem_2.patch serhiy.storchaka, 2014-09-11 08:19 review
tkinter_pymem_3.patch serhiy.storchaka, 2014-09-11 12:51 review
Messages (10)
msg226363 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-09-04 15:46
The PyMem_Malloc(size) function has a well defined behaviour: if size is 0, a pointer different than NULL is returned. It looks like ckalloc(size) returns NULL if the size is NULL: see issue #21951 (bug on AIX).

Moreover, memory allocated by ckalloc() is not traced by the new tracemalloc module!

Attached patch replaces calls to ckalloc() and ckfree() with PyMem_Malloc() and PyMem_Free(). It may fix the issue #21951 on AIX.

There is still a call to ckfree() in Tkapp_SplitList(). This memory block is allocated internally by Tcl, not directly by _tkinter.c.
msg226365 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-09-04 15:51
>        PyMem_Free(FREECAST argv);

FREECAST can be dropped here, PyMem_Free() always takes a void* pointer.
msg226366 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-09-04 17:08
Event structs (Tkapp_CallEvent, VarEvent, CommandEvent) must have been allocated by the caller using Tcl_Alloc or ckalloc (see man Tcl_ThreadQueueEvent).
msg226753 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-09-11 08:19
Here is updated patch which is synchronized with the tip after changes made in issue21951 and addresses my comments.
msg226764 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-09-11 11:18
I read tkinter_pymem_2.patch.

Remaining calls to ckalloc():

* they are only used to allocate events passed later to Tcl_ThreadQueueEvent(). Tcl_ThreadQueueEvent doc explicitly says that the memory must be allocated by Tcl_Alloc or ckalloc, so it's correct (PyMem cannot be used).

Remaining calls to ckfree():

* Tkapp_SplitList() calls ckfree() on memory allocated by Tcl_SplitList(), it's correct.

* Tkapp_CallDeallocArgs() ckfree() on memory allocated by PyMem_Malloc() => wrong (see my review on Rietveld).
msg226771 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-09-11 12:51
> * Tkapp_CallDeallocArgs() ckfree() on memory allocated by PyMem_Malloc() =>
> wrong 

Oh, you are right, thanks.

> (see my review on Rietveld).

Perhaps you forgot to press the "Publish" button.
msg226779 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-09-11 14:31
tkinter_pymem_3.patch looks good to me.
msg226782 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-09-11 15:42
And to me too. Please commit it, this is mainly your patch.
msg226784 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-09-11 15:52
New changeset 9f1d3e6e6ce6 by Victor Stinner in branch 'default':
Closes #22336: attemptckalloc() with PyMem_Malloc() in _tkinter
http://hg.python.org/cpython/rev/9f1d3e6e6ce6
msg226785 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-09-11 15:53
> And to me too. Please commit it, this is mainly your patch.

Ok, thanks, done.
History
Date User Action Args
2022-04-11 14:58:07adminsetgithub: 66532
2014-09-11 15:53:44vstinnersetmessages: + msg226785
2014-09-11 15:52:36python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg226784

resolution: fixed
stage: resolved
2014-09-11 15:42:17serhiy.storchakasetmessages: + msg226782
2014-09-11 14:32:00vstinnersetmessages: + msg226779
2014-09-11 12:51:12serhiy.storchakasetfiles: + tkinter_pymem_3.patch

messages: + msg226771
2014-09-11 11:18:09vstinnersetmessages: + msg226764
2014-09-11 08:19:47serhiy.storchakasetfiles: + tkinter_pymem_2.patch

messages: + msg226753
2014-09-04 17:08:02serhiy.storchakasetmessages: + msg226366
2014-09-04 15:51:51vstinnersetmessages: + msg226365
2014-09-04 15:46:15vstinnersetcomponents: + Tkinter
2014-09-04 15:46:10vstinnercreate