diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py
index 16ea17d024f24023690003c39b8fd92a63203c13..68a6620df3afcdd48358b2eb570be47009c1e0b7 100644
--- a/Cython/Compiler/ExprNodes.py
+++ b/Cython/Compiler/ExprNodes.py
@@ -318,6 +318,8 @@ class ExprNode(Node):
use_managed_ref = True # can be set by optimisation transforms
result_is_used = True
is_numpy_attribute = False
+ tracked_state = None
+ was_locked = True
# The Analyse Expressions phase for expressions is split
# into two sub-phases:
@@ -692,25 +694,45 @@ class ExprNode(Node):
error(self.pos, "Address is not constant")
def set_autorlock(self, env):
- self.entry.is_rlocked = True
- self.entry.needs_rlock = True
+ self.tracked_state.was_locked = True
+ self.tracked_state.is_rlocked = True
+ self.tracked_state.needs_rlock = True
def set_autowlock(self, env):
- print "Setting wlock"
- self.entry.is_wlocked = True
- self.entry.needs_wlock = True
+ self.tracked_state.was_locked = True
+ self.tracked_state.is_wlocked = True
+ self.tracked_state.needs_wlock = True
- def is_autolock(self, env):
+ def is_autolock(self):
return self.type.is_cyp_class and self.type.lock_mode == "autolock"
- def is_checklock(self, env):
+ def is_checklock(self):
return self.type.is_cyp_class and self.type.lock_mode == "checklock"
+ def get_tracked_state(self, env):
+ if not hasattr(self, 'entry') or not self.entry.type.is_cyp_class:
+ return
+ self.tracked_state = env.lookup_tracked(self.entry)
+ if self.tracked_state is None:
+ self.tracked_state = env.declare_tracked(self.entry)
+ if self.is_autolock():
+ env.declare_autolocked(self)
+ self.was_locked = self.tracked_state.was_locked
+ self.tracked_state.was_locked = True
+
def is_rhs_locked(self, env):
- return not(hasattr(self, 'entry') and self.entry.type.is_cyp_class and not (self.entry.is_rlocked or self.entry.is_wlocked))
+ if not hasattr(self, 'entry') or not self.entry.type.is_cyp_class:
+ # These nodes couldn't be tracked (because it is for example a constant),
+ # so we let them pass silently
+ return True
+ return self.tracked_state.is_rlocked or self.tracked_state.is_wlocked
def is_lhs_locked(self, env):
- return not(hasattr(self, 'entry') and self.entry.type.is_cyp_class and not self.entry.is_wlocked)
+ if not hasattr(self, 'entry') or not self.entry.type.is_cyp_class:
+ # These nodes couldn't be tracked (because it is for example a constant),
+ # so we let them pass silently
+ return True
+ return self.tracked_state.is_wlocked
def ensure_subexpr_rhs_locked(self, env):
for node in self.subexpr_nodes():
@@ -722,20 +744,24 @@ class ExprNode(Node):
def ensure_rhs_locked(self, env, is_dereferenced = False):
self.ensure_subexpr_rhs_locked(env)
+ if not self.tracked_state:
+ self.get_tracked_state(env)
if is_dereferenced:
if not self.is_rhs_locked(env):
- if self.is_checklock(env):
+ if self.is_checklock():
error(self.pos, "This expression is not correctly locked (read lock needed)")
- elif self.is_autolock(env):
+ elif self.is_autolock():
self.set_autorlock(env)
def ensure_lhs_locked(self, env, is_dereferenced = False):
self.ensure_subexpr_lhs_locked(env)
+ if not self.tracked_state:
+ self.get_tracked_state(env)
if is_dereferenced:
if not self.is_lhs_locked(env):
- if self.is_checklock(env):
+ if self.is_checklock():
error(self.pos, "This expression is not correctly locked (write lock needed)")
- elif self.is_autolock(env):
+ elif self.is_autolock():
self.set_autowlock(env)
# ----------------- Result Allocation -----------------
@@ -2314,8 +2340,14 @@ class NameNode(AtomicExprNode):
code.error_goto_if_null(self.result(), self.pos)))
code.put_gotref(self.py_result())
- elif entry.is_local and entry.type.is_cyp_class:
+ elif entry.type.is_cyp_class:
code.put_cygotref(self.result())
+ if not self.was_locked and self.is_autolock():
+ tracked_state = self.tracked_state
+ if tracked_state.needs_wlock:
+ code.putln("Cy_WLOCK(%s);" % self.result())
+ elif tracked_state.needs_rlock:
+ code.putln("Cy_RLOCK(%s);" % self.result())
#pass
# code.putln(entry.cname)
elif entry.is_local or entry.in_closure or entry.from_closure or entry.type.is_memoryviewslice:
@@ -2333,6 +2365,7 @@ class NameNode(AtomicExprNode):
exception_check=None, exception_value=None):
#print "NameNode.generate_assignment_code:", self.name ###
entry = self.entry
+ tracked_state = self.tracked_state
if entry is None:
return # There was an error earlier
@@ -2340,7 +2373,7 @@ class NameNode(AtomicExprNode):
and not self.lhs_of_first_assignment and not rhs.in_module_scope):
error(self.pos, "Literal list must be assigned to pointer at time of declaration")
- if entry.needs_wlock or entry.needs_rlock:
+ if self.is_autolock() and tracked_state and (tracked_state.needs_wlock or tracked_state.needs_rlock):
code.putln("Cy_UNLOCK(%s);" % self.result())
# is_pyglobal seems to be True for module level-globals only.
@@ -2445,10 +2478,11 @@ class NameNode(AtomicExprNode):
code.putln('new (&%s) decltype(%s){%s};' % (self.result(), self.result(), result))
elif result != self.result():
code.putln('%s = %s;' % (self.result(), result))
- if entry.needs_wlock:
- code.putln("Cy_WLOCK(%s);" % self.result())
- elif entry.needs_rlock:
- code.putln("Cy_RLOCK(%s);" % self.result())
+ if self.is_autolock():
+ if tracked_state.needs_wlock:
+ code.putln("Cy_WLOCK(%s);" % self.result())
+ elif tracked_state.needs_rlock:
+ code.putln("Cy_RLOCK(%s);" % self.result())
if debug_disposal_code:
print("NameNode.generate_assignment_code:")
print("...generating post-assignment code for %s" % rhs)
@@ -5827,10 +5861,11 @@ class SimpleCallNode(CallNode):
for i in range(min(max_nargs, actual_nargs)):
formal_arg = func_type.args[i]
actual_arg = args[i]
- if formal_arg.type.is_const:
- actual_arg.ensure_rhs_locked(env, is_dereferenced = True)
- else:
- actual_arg.ensure_lhs_locked(env, is_dereferenced = True)
+ if formal_arg.type.is_cyp_class:
+ if formal_arg.type.is_const:
+ actual_arg.ensure_rhs_locked(env, is_dereferenced = True)
+ else:
+ actual_arg.ensure_lhs_locked(env, is_dereferenced = True)
# Coerce arguments
some_args_in_temps = False
for i in range(min(max_nargs, actual_nargs)):
@@ -7369,6 +7404,12 @@ class AttributeNode(ExprNode):
'"Memoryview is not initialized");'
'%s'
'}' % (self.result(), code.error_goto(self.pos)))
+ elif self.is_autolock():
+ if not self.was_locked:
+ if self.tracked_state.needs_wlock:
+ code.putln("Cy_WLOCK(%s);" % self.result())
+ elif self.tracked_state.needs_rlock:
+ code.putln("Cy_RLOCK(%s);" % self.result())
else:
# result_code contains what is needed, but we may need to insert
# a check and raise an exception
@@ -7408,7 +7449,9 @@ class AttributeNode(ExprNode):
rhs.result_as(self.ctype())))
else:
select_code = self.result()
- if self.entry.needs_rlock or self.entry.needs_wlock:
+ # XXX - Greater to have a getter, right ?
+ tracked_state = self.tracked_state
+ if self.is_autolock() and tracked_state and (tracked_state.needs_rlock or tracked_state.needs_wlock):
code.putln("Cy_UNLOCK(%s);" % select_code)
if self.type.is_pyobject and self.use_managed_ref:
rhs.make_owned_reference(code)
@@ -7431,10 +7474,11 @@ class AttributeNode(ExprNode):
select_code,
rhs.result_as(self.ctype())))
#rhs.result()))
- if self.entry.needs_wlock:
- code.putln("Cy_WLOCK(%s);" % select_code)
- elif self.entry.needs_rlock:
- code.putln("Cy_RLOCK(%s);" % select_code)
+ if self.is_autolock():
+ if tracked_state.needs_wlock:
+ code.putln("Cy_WLOCK(%s);" % select_code)
+ elif tracked_state.needs_rlock:
+ code.putln("Cy_RLOCK(%s);" % select_code)
rhs.generate_post_assignment_code(code)
rhs.free_temps(code)
diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py
index 4c5ad999a7a0f44a5f849e2078bcd6f9c4d3fcd9..48c170a8336f2cbea307d5cf245c9e64fe753042 100644
--- a/Cython/Compiler/Nodes.py
+++ b/Cython/Compiler/Nodes.py
@@ -2168,6 +2168,23 @@ class FuncDefNode(StatNode, BlockNode):
# ----- Return cleanup for both error and no-error return
code.put_label(code.return_from_error_cleanup_label)
+ for node in reversed(lenv.autolocked_nodes):
+ # We iterate in the reverse order to properly unlock
+ # nested locked objects (aka most nested first).
+
+ # For example, if we have the following situation:
+ # obj.sub_obj.attr = some_value
+ # If obj and sub_obj are both of autolocked types,
+ # the obj (name)node is declared before the sub_obj (attribute)node.
+ # If we unlock first obj, another thread could immediately acquire
+ # a write lock and change where sub_obj points to.
+ # We would then try to unlock the new sub_obj reference,
+ # which leads to a dangling lock on the previous reference
+ # (and attempt to unlock a non-locked ref).
+
+ if not node.was_locked and (node.tracked_state.needs_wlock or node.tracked_state.needs_rlock):
+ code.putln("Cy_UNLOCK(%s);" % node.result())
+
for entry in lenv.var_entries:
if not entry.used or entry.in_closure:
continue
@@ -2186,10 +2203,6 @@ class FuncDefNode(StatNode, BlockNode):
# NULL as a valid cypclass (with a typecast)
code.put_cyxdecref(entry.cname)
- for node in lenv.autolocked_nodes:
- if node.entry.needs_rlock or node.entry.needs_wlock:
- code.putln("Cy_UNLOCK(%s);" % node.result())
-
# Decref any increfed args
for entry in lenv.arg_entries:
if entry.type.is_pyobject:
@@ -2585,7 +2598,8 @@ class CFuncDefNode(FuncDefNode):
entry = self.local_scope.declare(_name, _cname, _type, _pos, 'private')
entry.is_variable = 1
# Even if it is checklock it should be OK to mess with self without locking
- entry.is_wlocked = True
+ self_locking_state = self.local_scope.declare_tracked(entry)
+ self_locking_state.is_wlocked = True
def declare_cpdef_wrapper(self, env):
if self.overridable:
@@ -5550,11 +5564,6 @@ class SingleAssignmentNode(AssignmentNode):
self.lhs = self.lhs.analyse_target_types(env)
self.lhs.gil_assignment_check(env)
- if hasattr(self.lhs, 'entry'):
- entry = self.lhs.entry
- if entry.type.is_cyp_class and entry.type.lock_mode == "autolock"\
- and not (entry.needs_rlock or entry.needs_wlock):
- env.declare_autolocked(self.lhs)
self.rhs.ensure_rhs_locked(env)
self.lhs.ensure_lhs_locked(env)
unrolled_assignment = self.unroll_lhs(env)
@@ -8186,13 +8195,16 @@ class LockCypclassNode(StatNode):
def analyse_expressions(self, env):
self.obj = self.obj.analyse_types(env)
+ self.obj.ensure_rhs_locked(env)
if not hasattr(self.obj, 'entry'):
error(self.pos, "The (un)locking target has no entry")
if not self.obj.type.is_cyp_class:
error(self.pos, "Cannot (un)lock a non-cypclass variable !")
- is_rlocked = self.obj.entry.is_rlocked
- is_wlocked = self.obj.entry.is_wlocked
+ # FIXME: this is a bit redundant here
+ self.obj.get_tracked_state(env)
+ is_rlocked = self.obj.is_rhs_locked(env)
+ is_wlocked = self.obj.is_lhs_locked(env)
if self.state == "unclocked" and not (is_rlocked or is_wlocked):
error(self.pos, "Cannot unlock an already unlocked object !")
@@ -8209,23 +8221,29 @@ class LockCypclassNode(StatNode):
self.was_rlocked = is_rlocked
self.was_wlocked = is_wlocked
+ tracked_state = self.obj.tracked_state
+
if self.state == "rlocked":
- self.obj.entry.is_rlocked = True
- self.obj.entry.is_wlocked = False
+ tracked_state.is_rlocked = True
+ tracked_state.is_wlocked = False
elif self.state == "wlocked":
- self.obj.entry.is_rlocked = False
- self.obj.entry.is_wlocked = True
+ tracked_state.is_rlocked = False
+ tracked_state.is_wlocked = True
else:
- self.obj.entry.is_rlocked = False
- self.obj.entry.is_wlocked = False
+ tracked_state.is_rlocked = False
+ tracked_state.is_wlocked = False
self.body = self.body.analyse_expressions(env)
- self.obj.entry.is_rlocked = self.was_rlocked
- self.obj.entry.is_wlocked = self.was_wlocked
+ #self.obj.entry.is_rlocked = self.was_rlocked
+ #self.obj.entry.is_wlocked = self.was_wlocked
+
+ tracked_state.is_rlocked = self.was_rlocked
+ tracked_state.is_wlocked = self.was_wlocked
return self
def generate_execution_code(self, code):
+ self.obj.generate_evaluation_code(code)
# We must unlock if it's a 'with unlocked' statement,
# or if we're changing lock type.
if self.was_rlocked or self.was_wlocked:
diff --git a/Cython/Compiler/Symtab.py b/Cython/Compiler/Symtab.py
index 91ec8b6b763d3b1c05224b9234bad737a618698c..fa24128142b43a7452786d14dd40a05a3f6c92e7 100644
--- a/Cython/Compiler/Symtab.py
+++ b/Cython/Compiler/Symtab.py
@@ -139,6 +139,7 @@ class Entry(object):
# is_rlocked boolean Is locked with a read lock (used for cypclass)
# needs_rlock boolean The entry needs a read lock (used in autolock mode)
# needs_wlock boolean The entry needs a write lock (used in autolock mode)
+ # was_locked boolean Indicates to nodes falling through that the first lock already took place
# TODO: utility_code and utility_code_definition serves the same purpose...
@@ -213,6 +214,7 @@ class Entry(object):
is_rlocked = False
needs_rlock = False
needs_wlock = False
+ was_locked = False
def __init__(self, name, cname, type, pos = None, init = None):
self.name = name
@@ -283,6 +285,15 @@ class InnerEntry(Entry):
def all_entries(self):
return self.defining_entry.all_entries()
+class TrackedLockedEntry:
+ def __init__(self, entry, scope):
+ self.entry = entry
+ self.scope = scope
+ self.is_wlocked = False
+ self.is_rlocked = False
+ self.needs_wlock = False
+ self.needs_rlock = False
+ self.was_locked = False
class Scope(object):
# name string Unqualified name
@@ -365,6 +376,8 @@ class Scope(object):
self.buffer_entries = []
self.lambda_defs = []
self.id_counters = {}
+ self.tracked_entries = {}
+
def __deepcopy__(self, memo):
return self
@@ -447,6 +460,18 @@ class Scope(object):
for scope in sorted(self.subscopes, key=operator.attrgetter('scope_prefix')):
yield scope
+ def declare_tracked(self, entry):
+ # Keying only with the name is wrong: if we have multiple attributes
+ # with the same name in different cypclass, this will conflict.
+ key = entry
+ self.tracked_entries[key] = TrackedLockedEntry(entry, self)
+ return self.tracked_entries[key]
+
+ def lookup_tracked(self, entry):
+ # We don't chain up the scopes on purpose: we want to keep things local
+ key = entry
+ return self.tracked_entries.get(key, None)
+
def declare(self, name, cname, type, pos, visibility, shadow = 0, is_type = 0, create_wrapper = 0):
# Create new entry, and add to dictionary if
# name is not None. Reports a warning if already
@@ -1799,7 +1824,8 @@ class LocalScope(Scope):
entry.init = "0"
entry.is_arg = 1
if type.is_cyp_class and type.lock_mode == "autolock":
- entry.is_wlocked = True
+ arg_lock_state = self.declare_tracked(entry)
+ arg_lock_state.is_wlocked = True
#entry.borrowed = 1 # Not using borrowed arg refs for now
self.arg_entries.append(entry)
return entry