classification
Title: multiprocessing initializes flags dict unsafely
Type: Stage:
Components: Versions: Python 3.0, Python 2.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Rhamphoryncus, georg.brandl, jjt009, jnoller, roudkerk
Priority: normal Keywords: patch

Created on 2008-06-12 20:23 by Rhamphoryncus, last changed 2008-06-13 07:21 by Rhamphoryncus. This issue is now closed.

Files
File name Uploaded Description Edit
multiprocessing.diff jjt009, 2008-06-13 01:05 Patch for multiprocessing.c
Messages (5)
msg68081 - (view) Author: Adam Olsen (Rhamphoryncus) Date: 2008-06-12 20:23
multiprocessing.c currently has code like this:

	temp = PyDict_New();
	if (!temp)
		return;
	if (PyModule_AddObject(module, "flags", temp) < 0)
		return;

PyModule_AddObject consumes the reference to temp, so it could
conceivable be deleted before the rest of this function finishes.
msg68114 - (view) Author: James Thomas (jjt009) Date: 2008-06-13 01:05
I believe this patch solves the problem. 
I added the line
Py_DECREF(temp)
after the code block shown above. 
I also changed the line
if (PyDict_SetItemString(temp, #name, Py_BuildValue("i", name)) < 0) return
to
if (PyDict_SetItemString(PyObject_GetAttrString(module, "flags"), #name,
Py_BuildValue("i", name)) < 0) return
msg68117 - (view) Author: Adam Olsen (Rhamphoryncus) Date: 2008-06-13 01:48
This doesn't look right.  PyDict_SetItemString doesn't steal the
references passed to it, so your reference to flags will be leaked each
time.  Besides, I think it's a little cleaner to INCREF it before call
PyModule_AddObject, then DECREF it at any point you return.

Additionally, I've just noticed that the result of Py_BuildValue is
getting leaked.  It should be stored to a temporary, added to flags,
then the temporary should be released.
msg68129 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-06-13 06:57
I think the easiest way is to just call AddObject after inserting the
values.

I fixed this, and the leaking BuildValue references, in r64223.
msg68132 - (view) Author: Adam Olsen (Rhamphoryncus) Date: 2008-06-13 07:21
Aww, that's cheating.  (Why didn't I think of that?)
History
Date User Action Args
2008-06-13 07:21:49Rhamphoryncussetmessages: + msg68132
2008-06-13 06:57:48georg.brandlsetstatus: open -> closed
resolution: fixed
messages: + msg68129
nosy: + georg.brandl
2008-06-13 01:48:06Rhamphoryncussetmessages: + msg68117
2008-06-13 01:05:57jjt009setfiles: + multiprocessing.diff
keywords: + patch
messages: + msg68114
nosy: + jjt009
2008-06-12 20:30:37jnollersetnosy: + roudkerk, jnoller
2008-06-12 20:23:24Rhamphoryncuscreate