pyrox.dev
1From 1b77e35d848340f2c5f4c9b82965c25a0572d48f Mon Sep 17 00:00:00 2001
2From: Jeroen Demeyer <J.Demeyer@UGent.be>
3Date: Thu, 14 Feb 2019 10:02:41 +0100
4Subject: [PATCH] @cython.trashcan directive to enable the Python trashcan for
5 deallocations
6
7---
8 Cython/Compiler/ModuleNode.py | 10 +++
9 Cython/Compiler/Options.py | 2 +
10 Cython/Compiler/PyrexTypes.py | 8 +-
11 Cython/Compiler/Symtab.py | 18 +++-
12 Cython/Utility/ExtensionTypes.c | 43 ++++++++++
13 tests/run/trashcan.pyx | 148 ++++++++++++++++++++++++++++++++
14 6 files changed, 227 insertions(+), 2 deletions(-)
15 create mode 100644 tests/run/trashcan.pyx
16
17diff --git a/Cython/Compiler/ModuleNode.py b/Cython/Compiler/ModuleNode.py
18index 56845330d..3a3e8a956 100644
19--- a/Cython/Compiler/ModuleNode.py
20+++ b/Cython/Compiler/ModuleNode.py
21@@ -1443,6 +1443,7 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode):
22
23 is_final_type = scope.parent_type.is_final_type
24 needs_gc = scope.needs_gc()
25+ needs_trashcan = scope.needs_trashcan()
26
27 weakref_slot = scope.lookup_here("__weakref__") if not scope.is_closure_class_scope else None
28 if weakref_slot not in scope.var_entries:
29@@ -1481,6 +1482,11 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode):
30 # running this destructor.
31 code.putln("PyObject_GC_UnTrack(o);")
32
33+ if needs_trashcan:
34+ code.globalstate.use_utility_code(
35+ UtilityCode.load_cached("PyTrashcan", "ExtensionTypes.c"))
36+ code.putln("__Pyx_TRASHCAN_BEGIN(o, %s)" % slot_func_cname)
37+
38 # call the user's __dealloc__
39 self.generate_usr_dealloc_call(scope, code)
40
41@@ -1554,6 +1560,10 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode):
42 code.putln("(*Py_TYPE(o)->tp_free)(o);")
43 if freelist_size:
44 code.putln("}")
45+
46+ if needs_trashcan:
47+ code.putln("__Pyx_TRASHCAN_END")
48+
49 code.putln(
50 "}")
51
52diff --git a/Cython/Compiler/Options.py b/Cython/Compiler/Options.py
53index d03119fca..05a728135 100644
54--- a/Cython/Compiler/Options.py
55+++ b/Cython/Compiler/Options.py
56@@ -319,6 +319,7 @@ directive_types = {
57 'freelist': int,
58 'c_string_type': one_of('bytes', 'bytearray', 'str', 'unicode'),
59 'c_string_encoding': normalise_encoding_name,
60+ 'trashcan': bool,
61 'cpow': bool
62 }
63
64@@ -362,6 +363,7 @@ directive_scopes = { # defaults to available everywhere
65 'np_pythran': ('module',),
66 'fast_gil': ('module',),
67 'iterable_coroutine': ('module', 'function'),
68+ 'trashcan' : ('cclass',),
69 }
70
71
72diff --git a/Cython/Compiler/PyrexTypes.py b/Cython/Compiler/PyrexTypes.py
73index c309bd04b..9231130b5 100644
74--- a/Cython/Compiler/PyrexTypes.py
75+++ b/Cython/Compiler/PyrexTypes.py
76@@ -1129,6 +1129,7 @@ class PyObjectType(PyrexType):
77 is_extern = False
78 is_subclassed = False
79 is_gc_simple = False
80+ builtin_trashcan = False # builtin type using trashcan
81
82 def __str__(self):
83 return "Python object"
84@@ -1183,10 +1184,14 @@ class PyObjectType(PyrexType):
85
86
87 builtin_types_that_cannot_create_refcycles = set([
88- 'bool', 'int', 'long', 'float', 'complex',
89+ 'object', 'bool', 'int', 'long', 'float', 'complex',
90 'bytearray', 'bytes', 'unicode', 'str', 'basestring'
91 ])
92
93+builtin_types_with_trashcan = set([
94+ 'dict', 'list', 'set', 'frozenset', 'tuple', 'type',
95+])
96+
97
98 class BuiltinObjectType(PyObjectType):
99 # objstruct_cname string Name of PyObject struct
100@@ -1211,6 +1216,7 @@ class BuiltinObjectType(PyObjectType):
101 self.typeptr_cname = "(&%s)" % cname
102 self.objstruct_cname = objstruct_cname
103 self.is_gc_simple = name in builtin_types_that_cannot_create_refcycles
104+ self.builtin_trashcan = name in builtin_types_with_trashcan
105 if name == 'type':
106 # Special case the type type, as many C API calls (and other
107 # libraries) actually expect a PyTypeObject* for type arguments.
108diff --git a/Cython/Compiler/Symtab.py b/Cython/Compiler/Symtab.py
109index 7361a55ae..f0c311ba6 100644
110--- a/Cython/Compiler/Symtab.py
111+++ b/Cython/Compiler/Symtab.py
112@@ -2043,7 +2043,7 @@ class PyClassScope(ClassScope):
113 class CClassScope(ClassScope):
114 # Namespace of an extension type.
115 #
116- # parent_type CClassType
117+ # parent_type PyExtensionType
118 # #typeobj_cname string or None
119 # #objstruct_cname string
120 # method_table_cname string
121@@ -2087,6 +2087,22 @@ class CClassScope(ClassScope):
122 return not self.parent_type.is_gc_simple
123 return False
124
125+ def needs_trashcan(self):
126+ # If the trashcan directive is explicitly set to False,
127+ # unconditionally disable the trashcan.
128+ directive = self.directives.get('trashcan')
129+ if directive is False:
130+ return False
131+ # If the directive is set to True and the class has Python-valued
132+ # C attributes, then it should use the trashcan in tp_dealloc.
133+ if directive and self.has_cyclic_pyobject_attrs:
134+ return True
135+ # Use the trashcan if the base class uses it
136+ base_type = self.parent_type.base_type
137+ if base_type and base_type.scope is not None:
138+ return base_type.scope.needs_trashcan()
139+ return self.parent_type.builtin_trashcan
140+
141 def needs_tp_clear(self):
142 """
143 Do we need to generate an implementation for the tp_clear slot? Can
144diff --git a/Cython/Utility/ExtensionTypes.c b/Cython/Utility/ExtensionTypes.c
145index dc187ab49..f359165df 100644
146--- a/Cython/Utility/ExtensionTypes.c
147+++ b/Cython/Utility/ExtensionTypes.c
148@@ -119,6 +119,49 @@ static int __Pyx_PyType_Ready(PyTypeObject *t) {
149 return r;
150 }
151
152+/////////////// PyTrashcan.proto ///////////////
153+
154+// These macros are taken from https://github.com/python/cpython/pull/11841
155+// Unlike the Py_TRASHCAN_SAFE_BEGIN/Py_TRASHCAN_SAFE_END macros, they
156+// allow dealing correctly with subclasses.
157+
158+// This requires CPython version >= 2.7.4
159+// (or >= 3.2.4 but we don't support such old Python 3 versions anyway)
160+#if CYTHON_COMPILING_IN_CPYTHON && PY_VERSION_HEX >= 0x02070400
161+#define __Pyx_TRASHCAN_BEGIN_CONDITION(op, cond) \
162+ do { \
163+ PyThreadState *_tstate = NULL; \
164+ // If "cond" is false, then _tstate remains NULL and the deallocator
165+ // is run normally without involving the trashcan
166+ if (cond) { \
167+ _tstate = PyThreadState_GET(); \
168+ if (_tstate->trash_delete_nesting >= PyTrash_UNWIND_LEVEL) { \
169+ // Store the object (to be deallocated later) and jump past
170+ // Py_TRASHCAN_END, skipping the body of the deallocator
171+ _PyTrash_thread_deposit_object((PyObject*)(op)); \
172+ break; \
173+ } \
174+ ++_tstate->trash_delete_nesting; \
175+ }
176+ // The body of the deallocator is here.
177+#define __Pyx_TRASHCAN_END \
178+ if (_tstate) { \
179+ --_tstate->trash_delete_nesting; \
180+ if (_tstate->trash_delete_later && _tstate->trash_delete_nesting <= 0) \
181+ _PyTrash_thread_destroy_chain(); \
182+ } \
183+ } while (0);
184+
185+#define __Pyx_TRASHCAN_BEGIN(op, dealloc) __Pyx_TRASHCAN_BEGIN_CONDITION(op, \
186+ Py_TYPE(op)->tp_dealloc == (destructor)(dealloc))
187+
188+#else
189+// The trashcan is a no-op on other Python implementations
190+// or old CPython versions
191+#define __Pyx_TRASHCAN_BEGIN(op, dealloc)
192+#define __Pyx_TRASHCAN_END
193+#endif
194+
195 /////////////// CallNextTpDealloc.proto ///////////////
196
197 static void __Pyx_call_next_tp_dealloc(PyObject* obj, destructor current_tp_dealloc);
198diff --git a/tests/run/trashcan.pyx b/tests/run/trashcan.pyx
199new file mode 100644
200index 000000000..93a501ff8
201--- /dev/null
202+++ b/tests/run/trashcan.pyx
203@@ -0,0 +1,148 @@
204+# mode: run
205+
206+cimport cython
207+
208+
209+# Count number of times an object was deallocated twice. This should remain 0.
210+cdef int double_deallocations = 0
211+def assert_no_double_deallocations():
212+ global double_deallocations
213+ err = double_deallocations
214+ double_deallocations = 0
215+ assert not err
216+
217+
218+# Compute x = f(f(f(...(None)...))) nested n times and throw away the result.
219+# The real test happens when exiting this function: then a big recursive
220+# deallocation of x happens. We are testing two things in the tests below:
221+# that Python does not crash and that no double deallocation happens.
222+# See also https://github.com/python/cpython/pull/11841
223+def recursion_test(f, int n=2**20):
224+ x = None
225+ cdef int i
226+ for i in range(n):
227+ x = f(x)
228+
229+
230+@cython.trashcan(True)
231+cdef class Recurse:
232+ """
233+ >>> recursion_test(Recurse)
234+ >>> assert_no_double_deallocations()
235+ """
236+ cdef public attr
237+ cdef int deallocated
238+
239+ def __init__(self, x):
240+ self.attr = x
241+
242+ def __dealloc__(self):
243+ # Check that we're not being deallocated twice
244+ global double_deallocations
245+ double_deallocations += self.deallocated
246+ self.deallocated = 1
247+
248+
249+cdef class RecurseSub(Recurse):
250+ """
251+ >>> recursion_test(RecurseSub)
252+ >>> assert_no_double_deallocations()
253+ """
254+ cdef int subdeallocated
255+
256+ def __dealloc__(self):
257+ # Check that we're not being deallocated twice
258+ global double_deallocations
259+ double_deallocations += self.subdeallocated
260+ self.subdeallocated = 1
261+
262+
263+@cython.freelist(4)
264+@cython.trashcan(True)
265+cdef class RecurseFreelist:
266+ """
267+ >>> recursion_test(RecurseFreelist)
268+ >>> recursion_test(RecurseFreelist, 1000)
269+ >>> assert_no_double_deallocations()
270+ """
271+ cdef public attr
272+ cdef int deallocated
273+
274+ def __init__(self, x):
275+ self.attr = x
276+
277+ def __dealloc__(self):
278+ # Check that we're not being deallocated twice
279+ global double_deallocations
280+ double_deallocations += self.deallocated
281+ self.deallocated = 1
282+
283+
284+# Subclass of list => uses trashcan by default
285+# As long as https://github.com/python/cpython/pull/11841 is not fixed,
286+# this does lead to double deallocations, so we skip that check.
287+cdef class RecurseList(list):
288+ """
289+ >>> RecurseList(42)
290+ [42]
291+ >>> recursion_test(RecurseList)
292+ """
293+ def __init__(self, x):
294+ super().__init__((x,))
295+
296+
297+# Some tests where the trashcan is NOT used. When the trashcan is not used
298+# in a big recursive deallocation, the __dealloc__s of the base classs are
299+# only run after the __dealloc__s of the subclasses.
300+# We use this to detect trashcan usage.
301+cdef int base_deallocated = 0
302+cdef int trashcan_used = 0
303+def assert_no_trashcan_used():
304+ global base_deallocated, trashcan_used
305+ err = trashcan_used
306+ trashcan_used = base_deallocated = 0
307+ assert not err
308+
309+
310+cdef class Base:
311+ def __dealloc__(self):
312+ global base_deallocated
313+ base_deallocated = 1
314+
315+
316+# Trashcan disabled by default
317+cdef class Sub1(Base):
318+ """
319+ >>> recursion_test(Sub1, 100)
320+ >>> assert_no_trashcan_used()
321+ """
322+ cdef public attr
323+
324+ def __init__(self, x):
325+ self.attr = x
326+
327+ def __dealloc__(self):
328+ global base_deallocated, trashcan_used
329+ trashcan_used += base_deallocated
330+
331+
332+@cython.trashcan(True)
333+cdef class Middle(Base):
334+ cdef public foo
335+
336+
337+# Trashcan disabled explicitly
338+@cython.trashcan(False)
339+cdef class Sub2(Middle):
340+ """
341+ >>> recursion_test(Sub2, 1000)
342+ >>> assert_no_trashcan_used()
343+ """
344+ cdef public attr
345+
346+ def __init__(self, x):
347+ self.attr = x
348+
349+ def __dealloc__(self):
350+ global base_deallocated, trashcan_used
351+ trashcan_used += base_deallocated
352--
3532.39.0
354