[pypy-commit] cffi default: Fix the logic to handle ffi.gc(x) being called several times with equal
arigo
noreply at buildbot.pypy.org
Sat Jun 6 20:55:33 CEST 2015
Author: Armin Rigo <arigo at tunes.org>
Branch:
Changeset: r2163:ce83dce59b83
Date: 2015-06-06 20:56 +0200
http://bitbucket.org/cffi/cffi/changeset/ce83dce59b83/
Log: Fix the logic to handle ffi.gc(x) being called several times with
equal values of x. (PyPy 2.6.0's RPython version is already
correct, which I guess is why the problem was noticed.)
diff --git a/c/cgc.c b/c/cgc.c
--- a/c/cgc.c
+++ b/c/cgc.c
@@ -2,79 +2,121 @@
/* translated to C from cffi/gc_weakref.py */
-static PyObject *const_name_pop;
+static PyObject *gc_wref_remove(PyObject *ffi_wref_tup, PyObject *key)
+{
+ FFIObject *ffi;
+ PyObject *indexobj, *destructor, *cdata, *freelist, *result;
+ Py_ssize_t index;
-static PyObject *gc_wref_remove(PyObject *ffi_wref_data, PyObject *arg)
-{
- PyObject *destructor, *cdata, *x;
- PyObject *res = PyObject_CallMethodObjArgs(ffi_wref_data,
- const_name_pop, arg, NULL);
- if (res == NULL)
- return NULL;
+ /* here, tup is a 4-tuple (ffi, destructor, cdata, index) */
+ if (!PyTuple_Check(ffi_wref_tup))
+ goto oops; /* should never occur */
- assert(PyTuple_Check(res));
- destructor = PyTuple_GET_ITEM(res, 0);
- cdata = PyTuple_GET_ITEM(res, 1);
- x = PyObject_CallFunctionObjArgs(destructor, cdata, NULL);
- Py_DECREF(res);
- if (x == NULL)
- return NULL;
- Py_DECREF(x);
+ ffi = (FFIObject *)PyTuple_GET_ITEM(ffi_wref_tup, 0);
+ destructor = PyTuple_GET_ITEM(ffi_wref_tup, 1);
+ cdata = PyTuple_GET_ITEM(ffi_wref_tup, 2);
+ indexobj = PyTuple_GET_ITEM(ffi_wref_tup, 3);
- Py_INCREF(Py_None);
- return Py_None;
+ index = PyInt_AsSsize_t(indexobj);
+ if (index < 0)
+ goto oops; /* should never occur */
+
+ /* assert gc_wrefs[index] is key */
+ if (PyList_GET_ITEM(ffi->gc_wrefs, index) != key)
+ goto oops; /* should never occur */
+
+ /* gc_wrefs[index] = freelist */
+ /* transfer ownership of 'freelist' to 'gc_wrefs[index]' */
+ freelist = ffi->gc_wrefs_freelist;
+ PyList_SET_ITEM(ffi->gc_wrefs, index, freelist);
+
+ /* freelist = index */
+ ffi->gc_wrefs_freelist = indexobj;
+ Py_INCREF(indexobj);
+
+ /* destructor(cdata) */
+ result = PyObject_CallFunctionObjArgs(destructor, cdata, NULL);
+
+ Py_DECREF(key); /* free the reference that was in 'gc_wrefs[index]' */
+ return result;
+
+ oops:
+ PyErr_SetString(PyExc_SystemError, "cgc: internal inconsistency");
+ /* random leaks may follow */
+ return NULL;
}
static PyMethodDef remove_callback = {
"gc_wref_remove", (PyCFunction)gc_wref_remove, METH_O
};
-static PyObject *gc_weakrefs_build(FFIObject *ffi, CDataObject *cd,
+static PyObject *gc_weakrefs_build(FFIObject *ffi, CDataObject *cdata,
PyObject *destructor)
{
- PyObject *new_cdata, *ref = NULL, *tup = NULL;
+ PyObject *new_cdata, *ref = NULL, *tup = NULL, *remove_fn = NULL;
+ Py_ssize_t index;
+ PyObject *datalist;
if (ffi->gc_wrefs == NULL) {
/* initialize */
- PyObject *data;
-
- if (const_name_pop == NULL) {
- const_name_pop = PyText_InternFromString("pop");
- if (const_name_pop == NULL)
- return NULL;
- }
- data = PyDict_New();
- if (data == NULL)
+ datalist = PyList_New(0);
+ if (datalist == NULL)
return NULL;
- ffi->gc_wrefs = PyCFunction_New(&remove_callback, data);
- Py_DECREF(data);
- if (ffi->gc_wrefs == NULL)
- return NULL;
+ ffi->gc_wrefs = datalist;
+ assert(ffi->gc_wrefs_freelist == NULL);
+ ffi->gc_wrefs_freelist = Py_None;
+ Py_INCREF(Py_None);
}
- new_cdata = do_cast(cd->c_type, (PyObject *)cd);
+ /* new_cdata = self.ffi.cast(typeof(cdata), cdata) */
+ new_cdata = do_cast(cdata->c_type, (PyObject *)cdata);
if (new_cdata == NULL)
goto error;
- ref = PyWeakref_NewRef(new_cdata, ffi->gc_wrefs);
+ /* if freelist is None: */
+ datalist = ffi->gc_wrefs;
+ if (ffi->gc_wrefs_freelist == Py_None) {
+ /* index = len(gc_wrefs) */
+ index = PyList_GET_SIZE(datalist);
+ /* gc_wrefs.append(None) */
+ if (PyList_Append(datalist, Py_None) < 0)
+ goto error;
+ tup = Py_BuildValue("OOOn", ffi, destructor, cdata, index);
+ }
+ else {
+ /* index = freelist */
+ index = PyInt_AsSsize_t(ffi->gc_wrefs_freelist);
+ if (index < 0)
+ goto error; /* should not occur */
+ tup = PyTuple_Pack(4, ffi, destructor, cdata, ffi->gc_wrefs_freelist);
+ }
+ if (tup == NULL)
+ goto error;
+
+ remove_fn = PyCFunction_New(&remove_callback, tup);
+ if (remove_fn == NULL)
+ goto error;
+
+ ref = PyWeakref_NewRef(new_cdata, remove_fn);
if (ref == NULL)
goto error;
- tup = PyTuple_Pack(2, destructor, cd);
- if (tup == NULL)
- goto error;
+ /* freelist = gc_wrefs[index] (which is None if we just did append(None)) */
+ /* transfer ownership of 'datalist[index]' into gc_wrefs_freelist */
+ Py_DECREF(ffi->gc_wrefs_freelist);
+ ffi->gc_wrefs_freelist = PyList_GET_ITEM(datalist, index);
+ /* gc_wrefs[index] = ref */
+ /* transfer ownership of 'ref' into 'datalist[index]' */
+ PyList_SET_ITEM(datalist, index, ref);
+ Py_DECREF(remove_fn);
+ Py_DECREF(tup);
- /* the 'self' of the function 'gc_wrefs' is actually the data dict */
- if (PyDict_SetItem(PyCFunction_GET_SELF(ffi->gc_wrefs), ref, tup) < 0)
- goto error;
-
- Py_DECREF(tup);
- Py_DECREF(ref);
return new_cdata;
error:
Py_XDECREF(new_cdata);
Py_XDECREF(ref);
Py_XDECREF(tup);
+ Py_XDECREF(remove_fn);
return NULL;
}
diff --git a/c/ffi_obj.c b/c/ffi_obj.c
--- a/c/ffi_obj.c
+++ b/c/ffi_obj.c
@@ -23,7 +23,7 @@
struct FFIObject_s {
PyObject_HEAD
- PyObject *gc_wrefs;
+ PyObject *gc_wrefs, *gc_wrefs_freelist;
struct _cffi_parse_info_s info;
char ctx_is_static, ctx_is_nonempty;
builder_c_t types_builder;
@@ -51,6 +51,7 @@
return NULL;
}
ffi->gc_wrefs = NULL;
+ ffi->gc_wrefs_freelist = NULL;
ffi->info.ctx = &ffi->types_builder.ctx;
ffi->info.output = internal_output;
ffi->info.output_size = FFI_COMPLEXITY_OUTPUT;
@@ -63,6 +64,7 @@
{
PyObject_GC_UnTrack(ffi);
Py_XDECREF(ffi->gc_wrefs);
+ Py_XDECREF(ffi->gc_wrefs_freelist);
free_builder_c(&ffi->types_builder, ffi->ctx_is_static);
diff --git a/cffi/gc_weakref.py b/cffi/gc_weakref.py
--- a/cffi/gc_weakref.py
+++ b/cffi/gc_weakref.py
@@ -2,18 +2,27 @@
class GcWeakrefs(object):
- # code copied and adapted from WeakKeyDictionary.
-
def __init__(self, ffi):
self.ffi = ffi
- self.data = data = {}
- def remove(k):
- destructor, cdata = data.pop(k)
- destructor(cdata)
- self.remove = remove
+ self.data = []
+ self.freelist = None
def build(self, cdata, destructor):
# make a new cdata of the same type as the original one
new_cdata = self.ffi.cast(self.ffi._backend.typeof(cdata), cdata)
- self.data[ref(new_cdata, self.remove)] = destructor, cdata
+ #
+ def remove(key):
+ assert self.data[index] is key
+ self.data[index] = self.freelist
+ self.freelist = index
+ destructor(cdata)
+ #
+ key = ref(new_cdata, remove)
+ index = self.freelist
+ if index is None:
+ index = len(self.data)
+ self.data.append(key)
+ else:
+ self.freelist = self.data[index]
+ self.data[index] = key
return new_cdata
diff --git a/testing/cffi0/backend_tests.py b/testing/cffi0/backend_tests.py
--- a/testing/cffi0/backend_tests.py
+++ b/testing/cffi0/backend_tests.py
@@ -1457,6 +1457,63 @@
import gc; gc.collect(); gc.collect(); gc.collect()
assert seen == [1]
+ def test_gc_2(self):
+ ffi = FFI(backend=self.Backend())
+ p = ffi.new("int *", 123)
+ seen = []
+ q1 = ffi.gc(p, lambda p: seen.append(1))
+ q2 = ffi.gc(q1, lambda p: seen.append(2))
+ import gc; gc.collect()
+ assert seen == []
+ del q1, q2
+ import gc; gc.collect(); gc.collect(); gc.collect(); gc.collect()
+ assert seen == [2, 1]
+
+ def test_gc_3(self):
+ ffi = FFI(backend=self.Backend())
+ p = ffi.new("int *", 123)
+ r = ffi.new("int *", 123)
+ seen = []
+ seen_r = []
+ q1 = ffi.gc(p, lambda p: seen.append(1))
+ s1 = ffi.gc(r, lambda r: seen_r.append(4))
+ q2 = ffi.gc(q1, lambda p: seen.append(2))
+ s2 = ffi.gc(s1, lambda r: seen_r.append(5))
+ q3 = ffi.gc(q2, lambda p: seen.append(3))
+ import gc; gc.collect()
+ assert seen == []
+ assert seen_r == []
+ del q1, q2, q3, s2, s1
+ import gc; gc.collect(); gc.collect(); gc.collect(); gc.collect()
+ assert seen == [3, 2, 1]
+ assert seen_r == [5, 4]
+
+ def test_gc_4(self):
+ ffi = FFI(backend=self.Backend())
+ p = ffi.new("int *", 123)
+ seen = []
+ q1 = ffi.gc(p, lambda p: seen.append(1))
+ q2 = ffi.gc(q1, lambda p: seen.append(2))
+ q3 = ffi.gc(q2, lambda p: seen.append(3))
+ import gc; gc.collect()
+ assert seen == []
+ del q1, q3 # q2 remains, and has a hard ref to q1
+ import gc; gc.collect(); gc.collect(); gc.collect()
+ assert seen == [3]
+
+ def test_gc_finite_list(self):
+ ffi = FFI(backend=self.Backend())
+ p = ffi.new("int *", 123)
+ keepalive = []
+ for i in range(10):
+ keepalive.append(ffi.gc(p, lambda p: None))
+ assert len(ffi.gc_weakrefs.data) == i + 1 #should be a private attr
+ del keepalive[:]
+ import gc; gc.collect(); gc.collect()
+ for i in range(10):
+ keepalive.append(ffi.gc(p, lambda p: None))
+ assert len(ffi.gc_weakrefs.data) == 10
+
def test_CData_CType(self):
ffi = FFI(backend=self.Backend())
assert isinstance(ffi.cast("int", 0), ffi.CData)
diff --git a/testing/cffi1/test_new_ffi_1.py b/testing/cffi1/test_new_ffi_1.py
--- a/testing/cffi1/test_new_ffi_1.py
+++ b/testing/cffi1/test_new_ffi_1.py
@@ -1409,6 +1409,47 @@
import gc; gc.collect(); gc.collect(); gc.collect()
assert seen == [1]
+ def test_gc_2(self):
+ p = ffi.new("int *", 123)
+ seen = []
+ q1 = ffi.gc(p, lambda p: seen.append(1))
+ q2 = ffi.gc(q1, lambda p: seen.append(2))
+ import gc; gc.collect()
+ assert seen == []
+ del q1, q2
+ import gc; gc.collect(); gc.collect(); gc.collect(); gc.collect()
+ assert seen == [2, 1]
+
+ def test_gc_3(self):
+ p = ffi.new("int *", 123)
+ r = ffi.new("int *", 123)
+ seen = []
+ seen_r = []
+ q1 = ffi.gc(p, lambda p: seen.append(1))
+ s1 = ffi.gc(r, lambda r: seen_r.append(4))
+ q2 = ffi.gc(q1, lambda p: seen.append(2))
+ s2 = ffi.gc(s1, lambda r: seen_r.append(5))
+ q3 = ffi.gc(q2, lambda p: seen.append(3))
+ import gc; gc.collect()
+ assert seen == []
+ assert seen_r == []
+ del q1, q2, q3, s2, s1
+ import gc; gc.collect(); gc.collect(); gc.collect(); gc.collect()
+ assert seen == [3, 2, 1]
+ assert seen_r == [5, 4]
+
+ def test_gc_4(self):
+ p = ffi.new("int *", 123)
+ seen = []
+ q1 = ffi.gc(p, lambda p: seen.append(1))
+ q2 = ffi.gc(q1, lambda p: seen.append(2))
+ q3 = ffi.gc(q2, lambda p: seen.append(3))
+ import gc; gc.collect()
+ assert seen == []
+ del q1, q3 # q2 remains, and has a hard ref to q1
+ import gc; gc.collect(); gc.collect(); gc.collect()
+ assert seen == [3]
+
def test_CData_CType(self):
assert isinstance(ffi.cast("int", 0), ffi.CData)
assert isinstance(ffi.new("int *"), ffi.CData)
More information about the pypy-commit
mailing list