Skip to content

Commit c9d78aa

Browse files
tim-onetim-one
committed
Years in the making.
objimpl.h, pymem.h: Stop mapping PyMem_{Del, DEL} and PyMem_{Free, FREE} to PyObject_{Free, FREE} in a release build. They're aliases for the system free() now. _subprocess.c/sp_handle_dealloc(): Since the memory was originally obtained via PyObject_NEW, it must be released via PyObject_FREE (or _DEL). pythonrun.c, tokenizer.c, parsermodule.c: I lost count of the number of PyObject vs PyMem mismatches in these -- it's like the specific function called at each site was picked at random, sometimes even with memory obtained via PyMem getting released via PyObject. Changed most to use PyObject uniformly, since the blobs allocated are predictably small in most cases, and obmalloc is generally faster than system mallocs then. If extension modules in real life prove as sloppy as Python's front end, we'll have to revert the objimpl.h + pymem.h part of this patch. Note that no problems will show up in a debug build (all calls still go thru obmalloc then). Problems will show up only in a release build, most likely segfaults.
1 parent 1c168d8 commit c9d78aa

7 files changed

Lines changed: 72 additions & 63 deletions

File tree

‎Include/objimpl.h‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ PyAPI_FUNC(void) PyObject_Free(void *);
101101

102102
/* Macros */
103103
#ifdef WITH_PYMALLOC
104-
#ifdef PYMALLOC_DEBUG
104+
#ifdef PYMALLOC_DEBUG /* WITH_PYMALLOC && PYMALLOC_DEBUG */
105105
PyAPI_FUNC(void *) _PyObject_DebugMalloc(size_t nbytes);
106106
PyAPI_FUNC(void *) _PyObject_DebugRealloc(void *p, size_t nbytes);
107107
PyAPI_FUNC(void) _PyObject_DebugFree(void *p);
@@ -124,11 +124,7 @@ PyAPI_FUNC(void) _PyObject_DebugMallocStats(void);
124124
#else /* ! WITH_PYMALLOC */
125125
#define PyObject_MALLOC PyMem_MALLOC
126126
#define PyObject_REALLOC PyMem_REALLOC
127-
/* This is an odd one! For backward compatibility with old extensions, the
128-
PyMem "release memory" functions have to invoke the object allocator's
129-
free() function. When pymalloc isn't enabled, that leaves us using
130-
the platform free(). */
131-
#define PyObject_FREE free
127+
#define PyObject_FREE PyMem_FREE
132128

133129
#endif /* WITH_PYMALLOC */
134130

‎Include/pymem.h‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ PyAPI_FUNC(void) PyMem_Free(void *);
5959
/* Redirect all memory operations to Python's debugging allocator. */
6060
#define PyMem_MALLOC PyObject_MALLOC
6161
#define PyMem_REALLOC PyObject_REALLOC
62+
#define PyMem_FREE PyObject_FREE
6263

6364
#else /* ! PYMALLOC_DEBUG */
6465

@@ -68,14 +69,10 @@ PyAPI_FUNC(void) PyMem_Free(void *);
6869
pymalloc. To solve these problems, allocate an extra byte. */
6970
#define PyMem_MALLOC(n) malloc((n) ? (n) : 1)
7071
#define PyMem_REALLOC(p, n) realloc((p), (n) ? (n) : 1)
72+
#define PyMem_FREE free
7173

7274
#endif /* PYMALLOC_DEBUG */
7375

74-
/* In order to avoid breaking old code mixing PyObject_{New, NEW} with
75-
PyMem_{Del, DEL} and PyMem_{Free, FREE}, the PyMem "release memory"
76-
functions have to be redirected to the object deallocator. */
77-
#define PyMem_FREE PyObject_FREE
78-
7976
/*
8077
* Type-oriented memory interface
8178
* ==============================
@@ -95,11 +92,11 @@ PyAPI_FUNC(void) PyMem_Free(void *);
9592
#define PyMem_RESIZE(p, type, n) \
9693
( (p) = (type *) PyMem_REALLOC((p), (n) * sizeof(type)) )
9794

98-
/* In order to avoid breaking old code mixing PyObject_{New, NEW} with
99-
PyMem_{Del, DEL} and PyMem_{Free, FREE}, the PyMem "release memory"
100-
functions have to be redirected to the object deallocator. */
101-
#define PyMem_Del PyObject_Free
102-
#define PyMem_DEL PyObject_FREE
95+
/* PyMem{Del,DEL} are left over from ancient days, and shouldn't be used
96+
* anymore. They're just confusing aliases for PyMem_{Free,FREE} now.
97+
*/
98+
#define PyMem_Del PyMem_Free
99+
#define PyMem_DEL PyMem_FREE
103100

104101
#ifdef __cplusplus
105102
}

‎Misc/NEWS‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ Extension Modules
311311

312312
- Everything under lib-old was removed. This includes the following modules:
313313
Para, addpack, cmp, cmpcache, codehack, dircmp, dump, find, fmt, grep,
314-
lockfile, newdir, ni, packmail, poly, rand, statcache, tb, tzparse,
314+
lockfile, newdir, ni, packmail, poly, rand, statcache, tb, tzparse,
315315
util, whatsound, whrandom, zmod
316316

317317
- The following modules were removed: regsub, reconvert, regex, regex_syntax.
@@ -942,6 +942,17 @@ Build
942942
C API
943943
-----
944944

945+
- ``PyMem_{Del, DEL}`` and ``PyMem_{Free, FREE}`` no longer map to
946+
``PyObject_{Free, FREE}``. They map to the system ``free()`` now. If memory
947+
is obtained via the ``PyObject_`` family, it must be released via the
948+
``PyObject_`` family, and likewise for the ``PyMem_`` family. This has
949+
always been officially true, but when Python's small-object allocator was
950+
introduced, an attempt was made to cater to a few extension modules
951+
discovered at the time that obtained memory via ``PyObject_New`` but
952+
released it via ``PyMem_DEL``. It's years later, and if such code still
953+
exists it will fail now (probably with segfaults, but calling wrong
954+
low-level memory management functions can yield many symptoms).
955+
945956
- Added a C API for set and frozenset objects.
946957

947958
- Removed PyRange_New().

‎Modules/parsermodule.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -701,7 +701,7 @@ build_node_children(PyObject *tuple, node *root, int *line_num)
701701
}
702702
}
703703
len = PyString_GET_SIZE(temp) + 1;
704-
strn = (char *)PyMem_MALLOC(len);
704+
strn = (char *)PyObject_MALLOC(len);
705705
if (strn != NULL)
706706
(void) memcpy(strn, PyString_AS_STRING(temp), len);
707707
Py_DECREF(temp);
@@ -719,11 +719,11 @@ build_node_children(PyObject *tuple, node *root, int *line_num)
719719
}
720720
err = PyNode_AddChild(root, type, strn, *line_num, 0);
721721
if (err == E_NOMEM) {
722-
PyMem_DEL(strn);
722+
PyObject_FREE(strn);
723723
return (node *) PyErr_NoMemory();
724724
}
725725
if (err == E_OVERFLOW) {
726-
PyMem_DEL(strn);
726+
PyObject_FREE(strn);
727727
PyErr_SetString(PyExc_ValueError,
728728
"unsupported number of child nodes");
729729
return NULL;
@@ -742,7 +742,7 @@ build_node_children(PyObject *tuple, node *root, int *line_num)
742742
}
743743
Py_XDECREF(elem);
744744
}
745-
return (root);
745+
return root;
746746
}
747747

748748

@@ -787,7 +787,7 @@ build_node_tree(PyObject *tuple)
787787
if (res && encoding) {
788788
Py_ssize_t len;
789789
len = PyString_GET_SIZE(encoding) + 1;
790-
res->n_str = (char *)PyMem_MALLOC(len);
790+
res->n_str = (char *)PyObject_MALLOC(len);
791791
if (res->n_str != NULL)
792792
(void) memcpy(res->n_str, PyString_AS_STRING(encoding), len);
793793
Py_DECREF(encoding);

‎PC/_subprocess.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ sp_handle_dealloc(sp_handle_object* self)
104104
{
105105
if (self->handle != INVALID_HANDLE_VALUE)
106106
CloseHandle(self->handle);
107-
PyMem_DEL(self);
107+
PyObject_FREE(self);
108108
}
109109

110110
static PyMethodDef sp_handle_methods[] = {

0 commit comments

Comments
 (0)