- Python Home
- About
- News
- Documentation
- Downloads
- Community
- Foundation
- Developer's Guide
- Issue Tracker
- Issues
- Summaries
- User
- Administration
- Help
Issue1782
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.
Created on 2008-01-10 14:47 by hniksic, last changed 2022-04-11 14:56 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| addpatch | hniksic, 2008-01-14 08:39 | |||
| Messages (7) | |||
|---|---|---|---|
| msg59662 - (view) | Author: Hrvoje Nikšić (hniksic) * | Date: 2008-01-10 14:47 | |
PyModule_AddObject has somewhat strange reference-counting behavior in
that it *conditionally* steals a reference. In case of error it doesn't
change the reference to the passed object, but in case of success it
steals it. This means that, as written, PyModule_AddIntConstant and
PyModuleAddStringConstant can leak created objects if PyModule_AddObject
fails.
As far as I can tell, the correct way to write those functions would be
(using PyModule_AddIntConstant as the example):
int
PyModule_AddIntConstant(PyObject *m, const char *name, long value)
{
PyObject *o = PyInt_FromLong(value);
if (PyModule_AddObject(m, name, o) == 0)
return 0;
Py_XDECREF(o);
return -1;
}
PyModule_AddObject was obviously intended to enable writing the "simple"
code (it even gracefully handles being passed NULL object to add) like
the one in PyModule_AddIntConstant, but I don't see a way to enable such
usage and avoid both leaks and an interface change. Changing the
reference-counting behavior of PyModule_AddObject would be
backward-incompatible, but it might be a good idea to consider it for
Python 3.
If there is agreement that my analysis and the proposed fixes are
correct, I will produce a proper patch.
|
|||
| msg59664 - (view) | Author: Christian Heimes (christian.heimes) * ![]() |
Date: 2008-01-10 16:10 | |
Thanks for your analysis! I *think* you are right but I've to study the code more carefully before I can make a decision. We can't target the change for 2.5 though. Guido would accuse my of being insane again. *g* The problem should be discussed at the upcoming bug day. |
|||
| msg59673 - (view) | Author: Guido van Rossum (gvanrossum) * ![]() |
Date: 2008-01-10 17:31 | |
These are meant purely for the convenience of module initialization, and there correct handling of reference counts in the light of failures is of marginal importance. So while these may technically have leaks, you shouldn't care. |
|||
| msg59698 - (view) | Author: Hrvoje Nikšić (hniksic) * | Date: 2008-01-11 08:46 | |
I agree that a leak would very rarely occur in practice, but since there is a straightforward fix, why not apply it? If nothing else, the code in the core should be an example of writing leak-free Python/C code, and a fix will also prevent others from wasting time on this in the future. |
|||
| msg59716 - (view) | Author: Christian Heimes (christian.heimes) * ![]() |
Date: 2008-01-11 15:15 | |
I don't mind if you like to pursue the issue. I won't invest any time into it. But if you can come up with a patch we can surely apply it. |
|||
| msg59892 - (view) | Author: Hrvoje Nikšić (hniksic) * | Date: 2008-01-14 08:39 | |
Here is a patch, as per description above. |
|||
| msg60205 - (view) | Author: Georg Brandl (georg.brandl) * ![]() |
Date: 2008-01-19 18:02 | |
Committed in r60084. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:56:29 | admin | set | github: 46115 |
| 2008-01-19 18:02:57 | georg.brandl | set | status: open -> closed nosy: + georg.brandl resolution: fixed messages: + msg60205 |
| 2008-01-14 08:39:39 | hniksic | set | files:
+ addpatch messages: + msg59892 |
| 2008-01-11 15:15:31 | christian.heimes | set | messages: + msg59716 |
| 2008-01-11 08:46:39 | hniksic | set | messages: + msg59698 |
| 2008-01-10 17:31:40 | gvanrossum | set | nosy:
+ gvanrossum messages: + msg59673 |
| 2008-01-10 16:10:07 | christian.heimes | set | priority: normal nosy: + christian.heimes messages: + msg59664 versions: + Python 2.6, Python 3.0, - Python 2.5 |
| 2008-01-10 14:47:50 | hniksic | create | |


