[pypy-commit] pypy py3k: cpyext: Adapt cpython issue14325, already done in the 2.x branch.

amauryfa noreply at buildbot.pypy.org
Fri Sep 21 23:53:34 CEST 2012


Author: Amaury Forgeot d'Arc <amauryfa at gmail.com>
Branch: py3k
Changeset: r57466:a3a58be99c75
Date: 2012-09-21 23:43 +0200
http://bitbucket.org/pypy/pypy/changeset/a3a58be99c75/

Log:	cpyext: Adapt cpython issue14325, already done in the 2.x branch.
	This is a 3.3 feature, but pypy needs it to avoid the gc delay
	destruction of arguments.

diff --git a/pypy/module/cpyext/src/getargs.c b/pypy/module/cpyext/src/getargs.c
--- a/pypy/module/cpyext/src/getargs.c
+++ b/pypy/module/cpyext/src/getargs.c
@@ -22,16 +22,33 @@
 #define FLAG_COMPAT 1
 #define FLAG_SIZE_T 2
 
+typedef int (*destr_t)(PyObject *, void *);
+
+
+/* Keep track of "objects" that have been allocated or initialized and
+   which will need to be deallocated or cleaned up somehow if overall
+   parsing fails.
+*/
+typedef struct {
+  void *item;
+  destr_t destructor;
+} freelistentry_t;
+
+typedef struct {
+  int first_available;
+  freelistentry_t *entries;
+} freelist_t;
+
 
 /* Forward */
 static int vgetargs1(PyObject *, const char *, va_list *, int);
 static void seterror(int, const char *, int *, const char *, const char *);
 static char *convertitem(PyObject *, const char **, va_list *, int, int *,
-                         char *, size_t, PyObject **);
+                         char *, size_t, freelist_t *);
 static char *converttuple(PyObject *, const char **, va_list *, int,
-                          int *, char *, size_t, int, PyObject **);
+                          int *, char *, size_t, int, freelist_t *);
 static char *convertsimple(PyObject *, const char **, va_list *, int, char *,
-                           size_t, PyObject **);
+                           size_t, freelist_t *);
 static Py_ssize_t convertbuffer(PyObject *, void **p, char **);
 static int getbuffer(PyObject *, Py_buffer *, char**);
 
@@ -112,115 +129,54 @@
 
 /* Handle cleanup of allocated memory in case of exception */
 
-#define GETARGS_CAPSULE_NAME_CLEANUP_PTR "getargs.cleanup_ptr"
-#define GETARGS_CAPSULE_NAME_CLEANUP_BUFFER "getargs.cleanup_buffer"
-#define GETARGS_CAPSULE_NAME_CLEANUP_CONVERT "getargs.cleanup_convert"
-
-static void
-cleanup_ptr(PyObject *self)
+static int
+cleanup_ptr(PyObject *self, void *ptr)
 {
-    void *ptr = PyCapsule_GetPointer(self, GETARGS_CAPSULE_NAME_CLEANUP_PTR);
     if (ptr) {
         PyMem_FREE(ptr);
     }
-}
-
-static void
-cleanup_buffer(PyObject *self)
-{
-    Py_buffer *ptr = (Py_buffer *)PyCapsule_GetPointer(self, GETARGS_CAPSULE_NAME_CLEANUP_BUFFER);
-    if (ptr) {
-        PyBuffer_Release(ptr);
-    }
-}
-
-static int
-addcleanup(void *ptr, PyObject **freelist, int is_buffer)
-{
-    PyObject *cobj;
-    const char *name;
-    PyCapsule_Destructor destr;
-
-    if (is_buffer) {
-        destr = cleanup_buffer;
-        name = GETARGS_CAPSULE_NAME_CLEANUP_BUFFER;
-    } else {
-        destr = cleanup_ptr;
-        name = GETARGS_CAPSULE_NAME_CLEANUP_PTR;
-    }
-
-    if (!*freelist) {
-        *freelist = PyList_New(0);
-        if (!*freelist) {
-            destr(ptr);
-            return -1;
-        }
-    }
-
-    cobj = PyCapsule_New(ptr, name, destr);
-    if (!cobj) {
-        destr(ptr);
-        return -1;
-    }
-    if (PyList_Append(*freelist, cobj)) {
-        Py_DECREF(cobj);
-        return -1;
-    }
-    Py_DECREF(cobj);
-    return 0;
-}
-
-static void
-cleanup_convert(PyObject *self)
-{
-    typedef int (*destr_t)(PyObject *, void *);
-    destr_t destr = (destr_t)PyCapsule_GetContext(self);
-    void *ptr = PyCapsule_GetPointer(self,
-                                     GETARGS_CAPSULE_NAME_CLEANUP_CONVERT);
-    if (ptr && destr)
-        destr(NULL, ptr);
-}
-
-static int
-addcleanup_convert(void *ptr, PyObject **freelist, int (*destr)(PyObject*,void*))
-{
-    PyObject *cobj;
-    if (!*freelist) {
-        *freelist = PyList_New(0);
-        if (!*freelist) {
-            destr(NULL, ptr);
-            return -1;
-        }
-    }
-    cobj = PyCapsule_New(ptr, GETARGS_CAPSULE_NAME_CLEANUP_CONVERT,
-                         cleanup_convert);
-    if (!cobj) {
-        destr(NULL, ptr);
-        return -1;
-    }
-    if (PyCapsule_SetContext(cobj, destr) == -1) {
-        /* This really should not happen. */
-        Py_FatalError("capsule refused setting of context.");
-    }
-    if (PyList_Append(*freelist, cobj)) {
-        Py_DECREF(cobj); /* This will also call destr. */
-        return -1;
-    }
-    Py_DECREF(cobj);
     return 0;
 }
 
 static int
-cleanreturn(int retval, PyObject *freelist)
+cleanup_buffer(PyObject *self, void *ptr)
 {
-    if (freelist && retval != 0) {
-        /* We were successful, reset the destructors so that they
-           don't get called. */
-        Py_ssize_t len = PyList_GET_SIZE(freelist), i;
-        for (i = 0; i < len; i++)
-            PyCapsule_SetDestructor(PyList_GET_ITEM(freelist, i), NULL);
+    Py_buffer *buf = (Py_buffer *)ptr;
+    if (buf) {
+        PyBuffer_Release(buf);
     }
-    Py_XDECREF(freelist);
+    return 0;
+}
+
+static int
+addcleanup(void *ptr, freelist_t *freelist, destr_t destructor)
+{
+    int index;
+
+    index = freelist->first_available;
+    freelist->first_available += 1;
+
+    freelist->entries[index].item = ptr;
+    freelist->entries[index].destructor = destructor;
+
+    return 0;
+}
+
+static int
+cleanreturn(int retval, freelist_t *freelist)
+{
+    int index;
+
+    if (retval == 0) {
+      /* A failure occurred, therefore execute all of the cleanup
+         functions.
+      */
+      for (index = 0; index < freelist->first_available; ++index) {
+          freelist->entries[index].destructor(NULL,
+                                              freelist->entries[index].item);
+      }
+    }
+    PyMem_FREE(freelist->entries);
     return retval;
 }
 
@@ -239,7 +195,7 @@
     const char *formatsave = format;
     Py_ssize_t i, len;
     char *msg;
-    PyObject *freelist = NULL;
+    freelist_t freelist = {0, NULL};
     int compat = flags & FLAG_COMPAT;
 
     assert(compat || (args != (PyObject*)NULL));
@@ -295,6 +251,12 @@
 
     format = formatsave;
 
+    freelist.entries = PyMem_NEW(freelistentry_t, max);
+    if (freelist.entries == NULL) {
+        PyErr_NoMemory();
+        return 0;
+    }
+
     if (compat) {
         if (max == 0) {
             if (args == NULL)
@@ -304,7 +266,7 @@
                           fname==NULL ? "function" : fname,
                           fname==NULL ? "" : "()");
             PyErr_SetString(PyExc_TypeError, msgbuf);
-            return 0;
+            return cleanreturn(0, &freelist);
         }
         else if (min == 1 && max == 1) {
             if (args == NULL) {
@@ -313,26 +275,26 @@
                           fname==NULL ? "function" : fname,
                           fname==NULL ? "" : "()");
                 PyErr_SetString(PyExc_TypeError, msgbuf);
-                return 0;
+                return cleanreturn(0, &freelist);
             }
             msg = convertitem(args, &format, p_va, flags, levels,
                               msgbuf, sizeof(msgbuf), &freelist);
             if (msg == NULL)
-                return cleanreturn(1, freelist);
+                return cleanreturn(1, &freelist);
             seterror(levels[0], msg, levels+1, fname, message);
-            return cleanreturn(0, freelist);
+            return cleanreturn(0, &freelist);
         }
         else {
             PyErr_SetString(PyExc_SystemError,
                 "old style getargs format uses new features");
-            return 0;
+            return cleanreturn(0, &freelist);
         }
     }
 
     if (!PyTuple_Check(args)) {
         PyErr_SetString(PyExc_SystemError,
             "new style getargs format but argument is not a tuple");
-        return 0;
+        return cleanreturn(0, &freelist);
     }
 
     len = PyTuple_GET_SIZE(args);
@@ -352,7 +314,7 @@
             message = msgbuf;
         }
         PyErr_SetString(PyExc_TypeError, message);
-        return 0;
+        return cleanreturn(0, &freelist);
     }
 
     for (i = 0; i < len; i++) {
@@ -363,7 +325,7 @@
                           sizeof(msgbuf), &freelist);
         if (msg) {
             seterror(i+1, msg, levels, fname, msg);
-            return cleanreturn(0, freelist);
+            return cleanreturn(0, &freelist);
         }
     }
 
@@ -372,10 +334,10 @@
         *format != '|' && *format != ':' && *format != ';') {
         PyErr_Format(PyExc_SystemError,
                      "bad format string: %.200s", formatsave);
-        return cleanreturn(0, freelist);
+        return cleanreturn(0, &freelist);
     }
 
-    return cleanreturn(1, freelist);
+    return cleanreturn(1, &freelist);
 }
 
 
@@ -439,7 +401,7 @@
 static char *
 converttuple(PyObject *arg, const char **p_format, va_list *p_va, int flags,
              int *levels, char *msgbuf, size_t bufsize, int toplevel,
-             PyObject **freelist)
+             freelist_t *freelist)
 {
     int level = 0;
     int n = 0;
@@ -515,7 +477,7 @@
 
 static char *
 convertitem(PyObject *arg, const char **p_format, va_list *p_va, int flags,
-            int *levels, char *msgbuf, size_t bufsize, PyObject **freelist)
+            int *levels, char *msgbuf, size_t bufsize, freelist_t *freelist)
 {
     char *msg;
     const char *format = *p_format;
@@ -583,7 +545,7 @@
 
 static char *
 convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags,
-              char *msgbuf, size_t bufsize, PyObject **freelist)
+              char *msgbuf, size_t bufsize, freelist_t *freelist)
 {
     /* For # codes */
 #define FETCH_SIZE      int *q=NULL;Py_ssize_t *q2=NULL;\
@@ -848,7 +810,7 @@
             if (getbuffer(arg, (Py_buffer*)p, &buf) < 0)
                 return converterr(buf, arg, msgbuf, bufsize);
             format++;
-            if (addcleanup(p, freelist, 1)) {
+            if (addcleanup(p, freelist, cleanup_buffer)) {
                 return converterr(
                     "(cleanup problem)",
                     arg, msgbuf, bufsize);
@@ -894,7 +856,7 @@
                 if (getbuffer(arg, p, &buf) < 0)
                     return converterr(buf, arg, msgbuf, bufsize);
             }
-            if (addcleanup(p, freelist, 1)) {
+            if (addcleanup(p, freelist, cleanup_buffer)) {
                 return converterr(
                     "(cleanup problem)",
                     arg, msgbuf, bufsize);
@@ -1100,7 +1062,7 @@
                     PyErr_NoMemory();
                     RETURN_ERR_OCCURRED;
                 }
-                if (addcleanup(*buffer, freelist, 0)) {
+                if (addcleanup(*buffer, freelist, cleanup_ptr)) {
                     Py_DECREF(s);
                     return converterr(
                         "(cleanup problem)",
@@ -1142,7 +1104,7 @@
                 PyErr_NoMemory();
                 RETURN_ERR_OCCURRED;
             }
-            if (addcleanup(*buffer, freelist, 0)) {
+            if (addcleanup(*buffer, freelist, cleanup_ptr)) {
                 Py_DECREF(s);
                 return converterr("(cleanup problem)",
                                 arg, msgbuf, bufsize);
@@ -1203,7 +1165,7 @@
                 return converterr("(unspecified)",
                                   arg, msgbuf, bufsize);
             if (res == Py_CLEANUP_SUPPORTED &&
-                addcleanup_convert(addr, freelist, convert) == -1)
+                addcleanup(addr, freelist, convert) == -1)
                 return converterr("(cleanup problem)",
                                 arg, msgbuf, bufsize);
         }
@@ -1234,7 +1196,7 @@
             PyBuffer_Release((Py_buffer*)p);
             return converterr("contiguous buffer", arg, msgbuf, bufsize);
         }
-        if (addcleanup(p, freelist, 1)) {
+        if (addcleanup(p, freelist, cleanup_buffer)) {
             return converterr(
                 "(cleanup problem)",
                 arg, msgbuf, bufsize);
@@ -1422,7 +1384,8 @@
     const char *fname, *msg, *custom_msg, *keyword;
     int min = INT_MAX;
     int i, len, nargs, nkeywords;
-    PyObject *freelist = NULL, *current_arg;
+    PyObject *current_arg;
+    freelist_t freelist = {0, NULL};
 
     assert(args != NULL && PyTuple_Check(args));
     assert(keywords == NULL || PyDict_Check(keywords));
@@ -1446,6 +1409,12 @@
     for (len=0; kwlist[len]; len++)
         continue;
 
+    freelist.entries = PyMem_NEW(freelistentry_t, len);
+    if (freelist.entries == NULL) {
+        PyErr_NoMemory();
+        return 0;
+    }
+
     nargs = PyTuple_GET_SIZE(args);
     nkeywords = (keywords == NULL) ? 0 : PyDict_Size(keywords);
     if (nargs + nkeywords > len) {
@@ -1456,7 +1425,7 @@
                      len,
                      (len == 1) ? "" : "s",
                      nargs + nkeywords);
-        return 0;
+        return cleanreturn(0, &freelist);
     }
 
     /* convert tuple args and keyword args in same loop, using kwlist to drive process */
@@ -1470,7 +1439,7 @@
             PyErr_Format(PyExc_RuntimeError,
                          "More keyword list entries (%d) than "
                          "format specifiers (%d)", len, i);
-            return cleanreturn(0, freelist);
+            return cleanreturn(0, &freelist);
         }
         current_arg = NULL;
         if (nkeywords) {
@@ -1484,11 +1453,11 @@
                              "Argument given by name ('%s') "
                              "and position (%d)",
                              keyword, i+1);
-                return cleanreturn(0, freelist);
+                return cleanreturn(0, &freelist);
             }
         }
         else if (nkeywords && PyErr_Occurred())
-            return cleanreturn(0, freelist);
+            return cleanreturn(0, &freelist);
         else if (i < nargs)
             current_arg = PyTuple_GET_ITEM(args, i);
 
@@ -1497,7 +1466,7 @@
                 levels, msgbuf, sizeof(msgbuf), &freelist);
             if (msg) {
                 seterror(i+1, msg, levels, fname, custom_msg);
-                return cleanreturn(0, freelist);
+                return cleanreturn(0, &freelist);
             }
             continue;
         }
@@ -1506,14 +1475,14 @@
             PyErr_Format(PyExc_TypeError, "Required argument "
                          "'%s' (pos %d) not found",
                          keyword, i+1);
-            return cleanreturn(0, freelist);
+            return cleanreturn(0, &freelist);
         }
         /* current code reports success when all required args
          * fulfilled and no keyword args left, with no further
          * validation. XXX Maybe skip this in debug build ?
          */
         if (!nkeywords)
-            return cleanreturn(1, freelist);
+            return cleanreturn(1, &freelist);
 
         /* We are into optional args, skip thru to any remaining
          * keyword args */
@@ -1521,7 +1490,7 @@
         if (msg) {
             PyErr_Format(PyExc_RuntimeError, "%s: '%s'", msg,
                          format);
-            return cleanreturn(0, freelist);
+            return cleanreturn(0, &freelist);
         }
     }
 
@@ -1529,7 +1498,7 @@
         PyErr_Format(PyExc_RuntimeError,
             "more argument specifiers than keyword list entries "
             "(remaining format:'%s')", format);
-        return cleanreturn(0, freelist);
+        return cleanreturn(0, &freelist);
     }
 
     /* make sure there are no extraneous keyword arguments */
@@ -1542,7 +1511,7 @@
             if (!PyUnicode_Check(key)) {
                 PyErr_SetString(PyExc_TypeError,
                                 "keywords must be strings");
-                return cleanreturn(0, freelist);
+                return cleanreturn(0, &freelist);
             }
             /* check that _PyUnicode_AsString() result is not NULL */
             ks = _PyUnicode_AsString(key);
@@ -1559,12 +1528,12 @@
                              "'%U' is an invalid keyword "
                              "argument for this function",
                              key);
-                return cleanreturn(0, freelist);
+                return cleanreturn(0, &freelist);
             }
         }
     }
 
-    return cleanreturn(1, freelist);
+    return cleanreturn(1, &freelist);
 }
 
 
diff --git a/pypy/module/cpyext/test/test_getargs.py b/pypy/module/cpyext/test/test_getargs.py
--- a/pypy/module/cpyext/test/test_getargs.py
+++ b/pypy/module/cpyext/test/test_getargs.py
@@ -126,7 +126,7 @@
             PyBuffer_Release(&buf);
             return result;
             ''')
-        assert b'foo\0bar\0baz' == pybuffer('foo\0bar\0baz')
+        assert b'foo\0bar\0baz' == pybuffer(b'foo\0bar\0baz')
 
 
     def test_pyarg_parse_string_fails(self):
@@ -137,7 +137,6 @@
         pybuffer = self.import_parser(
             '''
             Py_buffer buf1, buf2, buf3;
-            PyObject *result;
             if (!PyArg_ParseTuple(args, "s*s*s*", &buf1, &buf2, &buf3)) {
                 return NULL;
             }
@@ -145,12 +144,11 @@
             return NULL;
             ''')
         freed = []
-        class freestring(str):
+        class freestring(bytes):
             def __del__(self):
                 freed.append('x')
         raises(TypeError, pybuffer,
-               freestring("string"), freestring("other string"), 42)
-        import gc; gc.collect()
+               freestring(b"string"), freestring(b"other string"), 42)
         assert freed == ['x', 'x']
 
 
@@ -169,4 +167,4 @@
             return PyString_FromStringAndSize(buf, len);
             ''')
         raises(TypeError, "charbuf(10)")
-        assert b'foo\0bar\0baz' == charbuf('foo\0bar\0baz')
+        assert b'foo\0bar\0baz' == charbuf(b'foo\0bar\0baz')


More information about the pypy-commit mailing list