From 86bb8a8feeb46eaecb0ca70d54dfe77c287c19d0 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Sun, 15 Mar 2009 19:34:39 +0100 Subject: [PATCH] refactored GIL checking into separate transform (ticket #205) --- Cython/Compiler/ExprNodes.py | 116 +++++++++++++++---------- Cython/Compiler/Main.py | 2 + Cython/Compiler/Nodes.py | 52 ++++++----- Cython/Compiler/ParseTreeTransforms.py | 38 +++++++- tests/errors/nogil.pyx | 7 +- 5 files changed, 139 insertions(+), 76 deletions(-) diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py index e6c3ed13..c6e3ddd9 100644 --- a/Cython/Compiler/ExprNodes.py +++ b/Cython/Compiler/ExprNodes.py @@ -331,6 +331,13 @@ class ExprNode(Node): def analyse_target_types(self, env): self.analyse_types(env) + def gil_check(self, env): + # By default, any expression based on Python objects is + # prevented in nogil environments. Subtypes must override + # this if they can work without the GIL. + if self.type.is_pyobject: + self._gil_check(env) + def gil_assignment_check(self, env): if env.nogil and self.type.is_pyobject: error(self.pos, "Assignment of Python object not allowed without gil") @@ -347,10 +354,6 @@ class ExprNode(Node): def addr_not_const(self): error(self.pos, "Address is not constant") - def gil_check(self, env): - if env is not None and env.nogil and self.type.is_pyobject: - self.gil_error() - # ----------------- Result Allocation ----------------- def result_in_temp(self): @@ -765,6 +768,7 @@ class NoneNode(PyConstNode): value = "Py_None" constant_result = None + gil_check = None def compile_time_value(self, denv): return None @@ -786,7 +790,8 @@ class ConstNode(AtomicNewTempExprNode): # value string C code fragment is_literal = 1 - + gil_check = None + def is_simple(self): return 1 @@ -816,6 +821,7 @@ class BoolNode(ConstNode): def calculate_result_code(self): return str(int(self.value)) + class NullNode(ConstNode): type = PyrexTypes.c_null_ptr_type value = "NULL" @@ -1003,12 +1009,9 @@ class LongNode(AtomicNewTempExprNode): def compile_time_value(self, denv): return long(self.value) - - gil_message = "Constructing Python long int" def analyse_types(self, env): self.type = py_object_type - self.gil_check(env) self.is_temp = 1 gil_message = "Constructing Python long int" @@ -1035,7 +1038,6 @@ class ImagNode(AtomicNewTempExprNode): def analyse_types(self, env): self.type = py_object_type - self.gil_check(env) self.is_temp = 1 gil_message = "Constructing complex number" @@ -1062,6 +1064,7 @@ class NameNode(AtomicExprNode): is_cython_module = False cython_attribute = None lhs_of_first_assignment = False + is_used_as_rvalue = 0 entry = None def create_analysed_rvalue(pos, env, entry): @@ -1177,8 +1180,17 @@ class NameNode(AtomicExprNode): self.is_temp = 0 else: self.is_temp = 1 + self.is_used_as_rvalue = 1 env.use_utility_code(get_name_interned_utility_code) - self.gil_check(env) + + def gil_check(self, env): + if self.is_used_as_rvalue: + entry = self.entry + if entry.is_builtin: + # if not Options.cache_builtins: # cached builtins are ok + self._gil_check(env) + elif entry.is_pyglobal: + self._gil_check(env) gil_message = "Accessing Python global or builtin" @@ -1407,7 +1419,6 @@ class BackquoteNode(ExprNode): self.arg.analyse_types(env) self.arg = self.arg.coerce_to_pyobject(env) self.type = py_object_type - self.gil_check(env) self.is_temp = 1 gil_message = "Backquote expression" @@ -1442,7 +1453,6 @@ class ImportNode(ExprNode): self.name_list.analyse_types(env) self.name_list.coerce_to_pyobject(env) self.type = py_object_type - self.gil_check(env) self.is_temp = 1 env.use_utility_code(import_utility_code) @@ -1478,7 +1488,6 @@ class IteratorNode(NewTempExprNode): self.sequence.analyse_types(env) self.sequence = self.sequence.coerce_to_pyobject(env) self.type = py_object_type - self.gil_check(env) self.is_temp = 1 gil_message = "Iterating over Python object" @@ -1745,7 +1754,6 @@ class IndexNode(ExprNode): else: self.index = self.index.coerce_to_pyobject(env) self.type = py_object_type - self.gil_check(env) self.is_temp = 1 else: if self.base.type.is_ptr or self.base.type.is_array: @@ -1763,6 +1771,11 @@ class IndexNode(ExprNode): "Invalid index type '%s'" % self.index.type) + def gil_check(self, env): + if not self.is_buffer_access: + if self.base.type.is_pyobject: + self._gil_check(env) + gil_message = "Indexing Python object" def check_const_addr(self): @@ -2009,9 +2022,9 @@ class SliceIndexNode(ExprNode): self.start = self.start.coerce_to(c_int, env) if self.stop: self.stop = self.stop.coerce_to(c_int, env) - self.gil_check(env) self.is_temp = 1 + gil_check = ExprNode._gil_check gil_message = "Slicing Python object" def generate_result_code(self, code): @@ -2197,7 +2210,6 @@ class SliceNode(ExprNode): self.stop = self.stop.coerce_to_pyobject(env) self.step = self.step.coerce_to_pyobject(env) self.type = py_object_type - self.gil_check(env) self.is_temp = 1 gil_message = "Constructing Python slice object" @@ -2214,11 +2226,6 @@ class SliceNode(ExprNode): class CallNode(NewTempExprNode): - def gil_check(self, env): - # Make sure we're not in a nogil environment - if env.nogil: - error(self.pos, "Calling gil-requiring function without gil") - def analyse_as_type_constructor(self, env): type = self.function.analyse_as_type(env) if type and type.is_struct_or_union: @@ -2234,6 +2241,15 @@ class CallNode(NewTempExprNode): self.coerce_to(type, env) return True + def gil_check(self, env): + func_type = self.function_type() + if func_type.is_pyobject: + self._gil_check(env) + elif not getattr(func_type, 'nogil', False): + self._gil_check(env) + + gil_message = "Calling gil-requiring function" + class SimpleCallNode(CallNode): # Function call without keyword, * or ** args. @@ -2303,7 +2319,6 @@ class SimpleCallNode(CallNode): self.arg_tuple.analyse_types(env) self.args = None self.type = py_object_type - self.gil_check(env) self.is_temp = 1 else: for arg in self.args: @@ -2381,9 +2396,6 @@ class SimpleCallNode(CallNode): if func_type.exception_check == '+': if func_type.exception_value is None: env.use_utility_code(cpp_exception_utility_code) - # Check gil - if not func_type.nogil: - self.gil_check(env) def calculate_result_code(self): return self.c_call_code() @@ -2501,6 +2513,8 @@ class GeneralCallNode(CallNode): subexprs = ['function', 'positional_args', 'keyword_args', 'starstar_arg'] + gil_check = CallNode._gil_check + def compile_time_value(self, denv): function = self.function.compile_time_value(denv) positional_args = self.positional_args.compile_time_value(denv) @@ -2534,7 +2548,6 @@ class GeneralCallNode(CallNode): self.starstar_arg = \ self.starstar_arg.coerce_to_pyobject(env) self.type = py_object_type - self.gil_check(env) self.is_temp = 1 def generate_result_code(self, code): @@ -2589,9 +2602,9 @@ class AsTupleNode(ExprNode): self.arg.analyse_types(env) self.arg = self.arg.coerce_to_pyobject(env) self.type = tuple_type - self.gil_check(env) self.is_temp = 1 + gil_check = ExprNode._gil_check gil_message = "Constructing Python tuple" def generate_result_code(self, code): @@ -2818,13 +2831,16 @@ class AttributeNode(NewTempExprNode): self.type = py_object_type self.is_py_attr = 1 self.interned_attr_cname = env.intern_identifier(self.attribute) - self.gil_check(env) else: if not obj_type.is_error: error(self.pos, "Object of type '%s' has no attribute '%s'" % (obj_type, self.attribute)) + def gil_check(self, env): + if self.is_py_attr: + self._gil_check(env) + gil_message = "Accessing Python attribute" def is_simple(self): @@ -2956,7 +2972,7 @@ class SequenceNode(NewTempExprNode): is_sequence_constructor = 1 unpacked_items = None - + def compile_time_value_list(self, denv): return [arg.compile_time_value(denv) for arg in self.args] @@ -2970,7 +2986,6 @@ class SequenceNode(NewTempExprNode): if not skip_children: arg.analyse_types(env) self.args[i] = arg.coerce_to_pyobject(env) self.type = py_object_type - self.gil_check(env) self.is_temp = 1 def analyse_target_types(self, env): @@ -3357,7 +3372,6 @@ class SetNode(NewTempExprNode): arg.analyse_types(env) self.args[i] = arg.coerce_to_pyobject(env) self.type = set_type - self.gil_check(env) self.is_temp = 1 def calculate_constant_result(self): @@ -3415,7 +3429,6 @@ class DictNode(ExprNode): self.type = dict_type for item in self.key_value_pairs: item.analyse_types(env) - self.gil_check(env) self.obj_conversion_errors = held_errors() release_errors(ignore=True) self.is_temp = 1 @@ -3505,6 +3518,8 @@ class DictItemNode(ExprNode): # value ExprNode subexprs = ['key', 'value'] + gil_check = None # handled by DictNode + def calculate_constant_result(self): self.constant_result = ( self.key.constant_result, self.value.constant_result) @@ -3553,7 +3568,6 @@ class ClassNode(ExprNode): self.doc = self.doc.coerce_to_pyobject(env) self.module_name = env.global_scope().qualified_name self.type = py_object_type - self.gil_check(env) self.is_temp = 1 env.use_utility_code(create_class_utility_code); @@ -3589,7 +3603,6 @@ class UnboundMethodNode(ExprNode): def analyse_types(self, env): self.function.analyse_types(env) self.type = py_object_type - self.gil_check(env) self.is_temp = 1 gil_message = "Constructing an unbound method" @@ -3612,7 +3625,6 @@ class PyCFunctionNode(AtomicNewTempExprNode): def analyse_types(self, env): self.type = py_object_type - self.gil_check(env) self.is_temp = 1 gil_message = "Constructing Python function" @@ -3673,7 +3685,6 @@ class UnopNode(ExprNode): if self.is_py_operation(): self.coerce_operand_to_pyobject(env) self.type = py_object_type - self.gil_check(env) self.is_temp = 1 else: self.analyse_c_operation(env) @@ -3683,7 +3694,11 @@ class UnopNode(ExprNode): def is_py_operation(self): return self.operand.type.is_pyobject - + + def gil_check(self, env): + if self.is_py_operation(): + self._gil_check(env) + def coerce_operand_to_pyobject(self, env): self.operand = self.operand.coerce_to_pyobject(env) @@ -3882,7 +3897,11 @@ class TypecastNode(NewTempExprNode): elif from_py and to_py: if self.typecheck and self.type.is_extension_type: self.operand = PyTypeTestNode(self.operand, self.type, env) - + + def gil_check(self, env): + if self.type.is_pyobject and self.is_temp: + self._gil_check(env) + def check_const(self): self.operand.check_const() @@ -4074,7 +4093,6 @@ class BinopNode(NewTempExprNode): if self.is_py_operation(): self.coerce_operands_to_pyobjects(env) self.type = py_object_type - self.gil_check(env) self.is_temp = 1 if Options.incref_local_binop and self.operand1.type.is_pyobject: self.operand1 = self.operand1.coerce_to_temp(env) @@ -4084,6 +4102,10 @@ class BinopNode(NewTempExprNode): def is_py_operation(self): return (self.operand1.type.is_pyobject or self.operand2.type.is_pyobject) + + def gil_check(self, env): + if self.is_py_operation(): + self._gil_check(env) def coerce_operands_to_pyobjects(self, env): self.operand1 = self.operand1.coerce_to_pyobject(env) @@ -4321,7 +4343,6 @@ class BoolBinopNode(NewTempExprNode): self.operand1 = self.operand1.coerce_to_pyobject(env) self.operand2 = self.operand2.coerce_to_pyobject(env) self.type = py_object_type - self.gil_check(env) else: self.operand1 = self.operand1.coerce_to_boolean(env) self.operand2 = self.operand2.coerce_to_boolean(env) @@ -4931,9 +4952,9 @@ class PyTypeTestNode(CoercionNode): assert dst_type.is_extension_type or dst_type.is_builtin_type, "PyTypeTest on non extension type" CoercionNode.__init__(self, arg) self.type = dst_type - self.gil_check(env) self.result_ctype = arg.ctype() + gil_check = CoercionNode._gil_check gil_message = "Python type test" def analyse_types(self, env): @@ -4974,14 +4995,13 @@ class CoerceToPyTypeNode(CoercionNode): def __init__(self, arg, env): CoercionNode.__init__(self, arg) self.type = py_object_type - self.gil_check(env) self.is_temp = 1 if not arg.type.to_py_function or not arg.type.create_convert_utility_code(env): error(arg.pos, "Cannot convert '%s' to Python object" % arg.type) - + gil_message = "Converting to Python object" - + def coerce_to_boolean(self, env): return self.arg.coerce_to_boolean(env).coerce_to_temp(env) @@ -5047,10 +5067,12 @@ class CoerceToBooleanNode(CoercionNode): CoercionNode.__init__(self, arg) self.type = PyrexTypes.c_bint_type if arg.type.is_pyobject: - if env.nogil: - self.gil_error() self.is_temp = 1 + def gil_check(self, env): + if self.arg.type.is_pyobject: + self._gil_check(env) + gil_message = "Truth-testing Python object" def check_const(self): @@ -5080,7 +5102,6 @@ class CoerceToTempNode(CoercionNode): self.type = self.arg.type self.is_temp = 1 if self.type.is_pyobject: - self.gil_check(env) self.result_ctype = py_object_type gil_message = "Creating temporary Python reference" @@ -5113,6 +5134,7 @@ class CloneNode(CoercionNode): # node is responsible for doing those things. subexprs = [] # Arg is not considered a subexpr + gil_check = None def __init__(self, arg): CoercionNode.__init__(self, arg) diff --git a/Cython/Compiler/Main.py b/Cython/Compiler/Main.py index 6fb86764..333d4aba 100644 --- a/Cython/Compiler/Main.py +++ b/Cython/Compiler/Main.py @@ -81,6 +81,7 @@ class Context(object): from ParseTreeTransforms import CreateClosureClasses, MarkClosureVisitor, DecoratorTransform from ParseTreeTransforms import InterpretCompilerDirectives, TransformBuiltinMethods from ParseTreeTransforms import ComprehensionTransform, AlignFunctionDefinitions + from ParseTreeTransforms import GilCheck from AutoDocTransforms import EmbedSignature from Optimize import FlattenInListTransform, SwitchTransform, IterationTransform from Optimize import FlattenBuiltinTypeCreation, ConstantFolding, FinalOptimizePhase @@ -129,6 +130,7 @@ class Context(object): IterationTransform(), SwitchTransform(), FinalOptimizePhase(self), + GilCheck(), # ClearResultCodes(self), # SpecialFunctions(self), # CreateClosureClasses(context), diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py index 0cd1e18a..9328dc85 100644 --- a/Cython/Compiler/Nodes.py +++ b/Cython/Compiler/Nodes.py @@ -134,7 +134,8 @@ class Node(object): gil_message = "Operation" - def gil_check(self, env): + gil_check = None + def _gil_check(self, env): if env.nogil: self.gil_error() @@ -539,9 +540,6 @@ class CFuncDeclaratorNode(CDeclaratorNode): # Catch attempted C-style func(void) decl if type.is_void: error(arg_node.pos, "Use spam() rather than spam(void) to declare a function with no arguments.") -# if type.is_pyobject and self.nogil: -# error(self.pos, -# "Function with Python argument cannot be declared nogil") func_type_args.append( PyrexTypes.CFuncTypeArg(name, type, arg_node.pos)) if arg_node.default: @@ -1378,18 +1376,20 @@ class CFuncDefNode(FuncDefNode): if not arg.name: error(arg.pos, "Missing argument name") self.declare_argument(env, arg) - + def need_gil_acquisition(self, lenv): + return self.type.with_gil + + def gil_check(self, env): type = self.type - with_gil = self.type.with_gil + with_gil = type.with_gil if type.nogil and not with_gil: if type.return_type.is_pyobject: error(self.pos, "Function with Python return type cannot be declared nogil") - for entry in lenv.var_entries + lenv.temp_entries: + for entry in env.var_entries + env.temp_entries: if entry.type.is_pyobject: error(self.pos, "Function declared nogil has Python locals or temporaries") - return with_gil def analyse_expressions(self, env): self.analyse_default_values(env) @@ -3227,8 +3227,8 @@ class PrintStatNode(StatNode): env.use_utility_code(printing_utility_code) if len(self.arg_tuple.args) == 1 and self.append_newline: env.use_utility_code(printing_one_utility_code) - self.gil_check(env) + gil_check = StatNode._gil_check gil_message = "Python print statement" def generate_execution_code(self, code): @@ -3272,8 +3272,8 @@ class ExecStatNode(StatNode): self.temp_result = env.allocate_temp_pyobject() env.release_temp(self.temp_result) env.use_utility_code(Builtin.pyexec_utility_code) - self.gil_check(env) + gil_check = StatNode._gil_check gil_message = "Python exec statement" def generate_execution_code(self, code): @@ -3311,12 +3311,15 @@ class DelStatNode(StatNode): def analyse_expressions(self, env): for arg in self.args: arg.analyse_target_expression(env, None) - if arg.type.is_pyobject: - self.gil_check(env) - else: + if not arg.type.is_pyobject: error(arg.pos, "Deletion of non-Python object") #arg.release_target_temp(env) + def gil_check(self, env): + for arg in self.args: + if arg.type.is_pyobject: + self._gil_check(env) + gil_message = "Deleting Python object" def generate_execution_code(self, code): @@ -3402,8 +3405,10 @@ class ReturnStatNode(StatNode): and not return_type.is_pyobject and not return_type.is_returncode): error(self.pos, "Return value required") - if return_type.is_pyobject: - self.gil_check(env) + + def gil_check(self, env): + if self.return_type.is_pyobject: + self._gil_check(env) gil_message = "Returning Python object" @@ -3478,8 +3483,8 @@ class RaiseStatNode(StatNode): self.exc_tb.release_temp(env) env.use_utility_code(raise_utility_code) env.use_utility_code(restore_exception_utility_code) - self.gil_check(env) + gil_check = StatNode._gil_check gil_message = "Raising exception" def generate_execution_code(self, code): @@ -3528,10 +3533,10 @@ class ReraiseStatNode(StatNode): child_attrs = [] def analyse_expressions(self, env): - self.gil_check(env) env.use_utility_code(raise_utility_code) env.use_utility_code(restore_exception_utility_code) + gil_check = StatNode._gil_check gil_message = "Raising exception" def generate_execution_code(self, code): @@ -3560,9 +3565,9 @@ class AssertStatNode(StatNode): self.cond.release_temp(env) if self.value: self.value.release_temp(env) - self.gil_check(env) #env.recycle_pending_temps() # TEMPORARY + gil_check = StatNode._gil_check gil_message = "Raising exception" def generate_execution_code(self, code): @@ -4069,9 +4074,8 @@ class TryExceptStatNode(StatNode): except_clause.analyse_declarations(env) if self.else_clause: self.else_clause.analyse_declarations(env) - self.gil_check(env) env.use_utility_code(reset_exception_utility_code) - + def analyse_expressions(self, env): self.body.analyse_expressions(env) self.cleanup_list = env.free_temp_entries[:] @@ -4085,8 +4089,8 @@ class TryExceptStatNode(StatNode): self.has_default_clause = default_clause_seen if self.else_clause: self.else_clause.analyse_expressions(env) - self.gil_check(env) + gil_check = StatNode._gil_check gil_message = "Try-except statement" def generate_execution_code(self, code): @@ -4368,8 +4372,8 @@ class TryFinallyStatNode(StatNode): self.body.analyse_expressions(env) self.cleanup_list = env.free_temp_entries[:] self.finally_clause.analyse_expressions(env) - self.gil_check(env) + gil_check = StatNode._gil_check gil_message = "Try-finally statement" def generate_execution_code(self, code): @@ -4516,8 +4520,8 @@ class GILStatNode(TryFinallyStatNode): # 'with gil' or 'with nogil' statement # # state string 'gil' or 'nogil' - - child_attrs = [] + +# child_attrs = [] preserve_exception = 0 diff --git a/Cython/Compiler/ParseTreeTransforms.py b/Cython/Compiler/ParseTreeTransforms.py index 3f92d738..3c3383db 100644 --- a/Cython/Compiler/ParseTreeTransforms.py +++ b/Cython/Compiler/ParseTreeTransforms.py @@ -845,7 +845,43 @@ class CreateClosureClasses(CythonTransform): def visit_FuncDefNode(self, node): self.create_class_from_scope(node, self.module_scope) return node - + + +class GilCheck(VisitorTransform): + """ + Call `node.gil_check(env)` on each node to make sure we hold the + GIL when we need it. Raise an error when on Python operations + inside a `nogil` environment. + """ + def __call__(self, root): + self.env_stack = [root.scope] + return super(GilCheck, self).__call__(root) + + def visit_FuncDefNode(self, node): + self.env_stack.append(node.local_scope) + if node.gil_check is not None: + node.gil_check(self.env_stack[-1]) + self.visitchildren(node) + self.env_stack.pop() + return node + + def visit_GILStatNode(self, node): + # FIXME: should we do some kind of GIL checking here, too? + # if node.gil_check is not None: + # node.gil_check(self.env_stack[-1]) + env = self.env_stack[-1] + was_nogil = env.nogil + env.nogil = node.state == 'nogil' + self.visitchildren(node) + env.nogil = was_nogil + return node + + def visit_Node(self, node): + if self.env_stack and node.gil_check is not None: + node.gil_check(self.env_stack[-1]) + self.visitchildren(node) + return node + class EnvTransform(CythonTransform): """ diff --git a/tests/errors/nogil.pyx b/tests/errors/nogil.pyx index b6d5035e..04feaa2d 100644 --- a/tests/errors/nogil.pyx +++ b/tests/errors/nogil.pyx @@ -82,14 +82,14 @@ _ERRORS = u""" 6: 6: Assignment of Python object not allowed without gil 4: 5: Function declared nogil has Python locals or temporaries 11: 5: Function with Python return type cannot be declared nogil -15: 5: Calling gil-requiring function without gil -24: 9: Calling gil-requiring function without gil +15: 5: Calling gil-requiring function not allowed without gil +24: 9: Calling gil-requiring function not allowed without gil 26:12: Assignment of Python object not allowed without gil 28: 8: Constructing complex number not allowed without gil 29:12: Accessing Python global or builtin not allowed without gil 30: 8: Backquote expression not allowed without gil -31:15: Python import not allowed without gil 31:15: Assignment of Python object not allowed without gil +31:15: Python import not allowed without gil 32:13: Python import not allowed without gil 32:25: Constructing Python list not allowed without gil 33:17: Iterating over Python object not allowed without gil @@ -126,7 +126,6 @@ _ERRORS = u""" 54: 8: Raising exception not allowed without gil 55:14: Truth-testing Python object not allowed without gil 57:17: Truth-testing Python object not allowed without gil -59: 8: Converting to Python object not allowed without gil 61: 8: Try-except statement not allowed without gil 65: 8: Try-finally statement not allowed without gil """ -- 2.26.2