Skip to content

C
cython
Commit 981572f1 authored by gsamain's avatar gsamain
Browse files
Options

Tracked states don't go outside of scope

parent 4e338f74
Show whitespace changes
Inline Side-by-side
Showing with 137 additions and 49 deletions
Cython/Compiler/ExprNodes.py
View file @ 981572f1
......@@ -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,9 +2478,10 @@ 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:
if self.is_autolock():
if tracked_state.needs_wlock:
code.putln("Cy_WLOCK(%s);" % self.result())
elif entry.needs_rlock:
elif tracked_state.needs_rlock:
code.putln("Cy_RLOCK(%s);" % self.result())
if debug_disposal_code:
print("NameNode.generate_assignment_code:")
......@@ -5827,6 +5861,7 @@ 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_cyp_class:
if formal_arg.type.is_const:
actual_arg.ensure_rhs_locked(env, is_dereferenced = True)
else:
......@@ -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,9 +7474,10 @@ class AttributeNode(ExprNode):
select_code,
rhs.result_as(self.ctype())))
#rhs.result()))
if self.entry.needs_wlock:
if self.is_autolock():
if tracked_state.needs_wlock:
code.putln("Cy_WLOCK(%s);" % select_code)
elif self.entry.needs_rlock:
elif tracked_state.needs_rlock:
code.putln("Cy_RLOCK(%s);" % select_code)
rhs.generate_post_assignment_code(code)
......
Cython/Compiler/Nodes.py
View file @ 981572f1
......@@ -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:
......
Cython/Compiler/Symtab.py
View file @ 981572f1
......@@ -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
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7