Introduce 'locked' qualifier and refactor locking

Cython/Compiler/Builtin.py

@@ -415,7 +415,7 @@ def inject_acthon_interfaces(self):
     init_scope(result_scope)
     acthon_result_type = result_type = PyrexTypes.CypClassType(
                 "ActhonResultInterface", result_scope, "ActhonResultInterface", (PyrexTypes.cy_object_type,),
-                lock_mode="autolock", activable=False)
+                activable=False)
     result_scope.type = result_type
     #result_type.set_scope is required because parent_type is used when doing scope inheritance
     result_type.set_scope(result_scope)
@@ -474,7 +474,7 @@ def inject_acthon_interfaces(self):
     init_scope(message_scope)
     acthon_message_type = message_type = PyrexTypes.CypClassType(
                 "ActhonMessageInterface", message_scope, "ActhonMessageInterface", (PyrexTypes.cy_object_type,),
-                lock_mode="autolock", activable=False)
+                activable=False)
     message_type.set_scope(message_scope)
     message_scope.type = message_type
 
@@ -488,7 +488,7 @@ def inject_acthon_interfaces(self):
     init_scope(sync_scope)
     acthon_sync_type = sync_type = PyrexTypes.CypClassType(
                 "ActhonSyncInterface", sync_scope, "ActhonSyncInterface", (PyrexTypes.cy_object_type,),
-                lock_mode="autolock", activable=False)
+                activable=False)
     sync_type.set_scope(sync_scope)
     sync_scope.type = sync_type
     sync_entry = self.declare("ActhonSyncInterface", "ActhonSyncInterface", sync_type, None, "extern")
@@ -557,7 +557,7 @@ def inject_acthon_interfaces(self):
     init_scope(queue_scope)
     acthon_queue_type = queue_type = PyrexTypes.CypClassType(
                 "ActhonQueueInterface", queue_scope, "ActhonQueueInterface", (PyrexTypes.cy_object_type,),
-                lock_mode="autolock", activable=False)
+                activable=False)
     queue_type.set_scope(queue_scope)
     queue_scope.type = queue_type
     queue_entry = self.declare("ActhonQueueInterface", "ActhonQueueInterface", queue_type, self, "extern")
@@ -594,7 +594,7 @@ def inject_acthon_interfaces(self):
     init_scope(activable_scope)
     acthon_activable_type = activable_type = PyrexTypes.CypClassType(
                 "ActhonActivableClass", activable_scope, "ActhonActivableClass", (PyrexTypes.cy_object_type,),
-                lock_mode="autolock", activable=False)
+                activable=False)
     activable_type.set_scope(activable_scope)
     activable_entry = self.declare("ActhonActivableClass", None, activable_type, "ActhonActivableClass", "extern")
     activable_entry.is_type = 1

Cython/Compiler/ExprNodes.py

@@ -7310,6 +7310,7 @@ class AttributeNode(ExprNode):
         return node
 
     def analyse_types(self, env, target = 0):
+        self.is_target = target
         self.initialized_check = env.directives['initializedcheck']
         node = self.analyse_as_cimported_attribute_node(env, target)
         if node is None and not target:
@@ -14261,10 +14262,10 @@ class CoerceToTempNode(CoercionNode):
 class CoerceToLockedNode(CoercionNode):
     # This node is used to lock a node of cypclass type around the evaluation of its subexpressions.
 
-    # rlock_only    boolean
+    # exclusive    boolean
 
-    def __init__(self, arg, env=None, rlock_only=False):
-        self.rlock_only = rlock_only
+    def __init__(self, arg, env=None, exclusive=True):
+        self.exclusive = exclusive
         self.type = arg.type
         arg = arg.coerce_to_temp(env)
         arg.postpone_subexpr_disposal = True
@@ -14295,13 +14296,13 @@ class CoerceToLockedNode(CoercionNode):
         # Create a scope to use scope bound resource management (RAII).
         code.putln("{")
 
-        # Since each lock guard has its onw scope,
+        # Since each lock guard has its own scope,
         # a prefix is enough to prevent name collisions.
         guard_code = "%sguard" % Naming.cypclass_lock_guard_prefix
-        if self.rlock_only:
-            code.putln("Cy_rlock_guard %s(%s, %s);" % (guard_code, self.result(), context))
-        else:
+        if self.exclusive:
             code.putln("Cy_wlock_guard %s(%s, %s);" % (guard_code, self.result(), context))
+        else:
+            code.putln("Cy_rlock_guard %s(%s, %s);" % (guard_code, self.result(), context))
 
     def generate_disposal_code(self, code):
         # Close the scope to release the lock.

Cython/Compiler/ModuleNode.py

@@ -1209,13 +1209,6 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode):
                 reified_call_args_list.append(Naming.optional_args_cname)
 
             # Locking CyObjects
-            # Here we completely ignore the lock mode (nolock/checklock/autolock)
-            # because the mode is used for direct calls, when the user have the possibility
-            # to manually lock or let the compiler handle it.
-            # Here, the user cannot lock manually, so we're taking the lock automatically.
-            #put_cypclass_op_on_narg_optarg(lambda arg: "Cy_RLOCK" if arg.type.is_const else "Cy_WLOCK",
-            #                               reified_function_entry.type, Naming.optional_args_cname, code)
-
             func_type = reified_function_entry.type
             opt_arg_name = Naming.optional_args_cname
             trylock_result = "trylock_result"

Cython/Compiler/Nodes.py

@@ -1559,7 +1559,6 @@ class CppClassNode(CStructOrUnionDefNode, BlockNode):
     #  templates     [(string, bool)] or None
     #  decorators    [DecoratorNode] or None
     #  cypclass      boolean
-    #  lock_mode     'nolock', 'checklock', 'autolock', or None
     #  activable     boolean
 
     decorators = None
@@ -1584,7 +1583,7 @@ class CppClassNode(CStructOrUnionDefNode, BlockNode):
         self.entry = env.declare_cpp_class(
             self.name, None, self.pos, self.cname,
             base_classes=[], visibility=self.visibility, templates=template_types,
-            cypclass=self.cypclass, lock_mode=self.lock_mode, activable=self.activable)
+            cypclass=self.cypclass, activable=self.activable)
 
     def analyse_declarations(self, env):
         if self.templates is None:
@@ -1642,7 +1641,7 @@ class CppClassNode(CStructOrUnionDefNode, BlockNode):
         self.entry = env.declare_cpp_class(
             self.name, scope, self.pos,
             self.cname, base_class_types, visibility=self.visibility, templates=template_types,
-            cypclass=self.cypclass, lock_mode=self.lock_mode, activable=self.activable)
+            cypclass=self.cypclass, activable=self.activable)
         if self.entry is None:
             return
         self.entry.is_cpp_class = 1
@@ -8615,8 +8614,11 @@ class LockCypclassNode(StatNode):
         self.obj.analyse_declarations(env)
 
     def analyse_expressions(self, env):
-        self.obj = self.obj.analyse_types(env)
+        self.obj = obj = self.obj.analyse_types(env)
         self.body = self.body.analyse_expressions(env)
+        if not obj.type.is_cyp_class:
+            error(obj.pos, "Locking non-cypclass reference")
+            return self
         return self
 
     def generate_execution_code(self, code):

Cython/Compiler/Parsing.py

@@ -2544,7 +2544,7 @@ def p_c_simple_base_type(s, self_flag, nonempty, templates = None):
             base_type=base_type, is_const=is_const, is_volatile=is_volatile)
 
     # Handle cypclass qualifiers
-    if s.sy == 'IDENT' and s.systring in ('active', 'iso'):
+    if s.sy == 'IDENT' and s.systring in ('active', 'iso', 'locked'):
         qualifier = s.systring
         s.next()
         base_type = p_c_base_type(s, self_flag=self_flag, nonempty=nonempty, templates=templates)
@@ -3864,10 +3864,8 @@ def p_cpp_class_definition(s, pos,  ctx):
     if s.sy == '[':
         error(s.position(), "Name options not allowed for C++ class")
     nogil = p_nogil(s) or cypclass
-    lock_mode = None
     activable = False
     if cypclass:
-        lock_mode = p_cypclass_lock_mode(s)
         activable = p_cypclass_activable(s)
     if s.sy == ':':
         s.next()
@@ -3897,7 +3895,7 @@ def p_cpp_class_definition(s, pos,  ctx):
         visibility = ctx.visibility,
         in_pxd = ctx.level == 'module_pxd',
         attributes = attributes,
-        templates = templates, cypclass=cypclass, lock_mode=lock_mode, activable=activable)
+        templates = templates, cypclass=cypclass, activable=activable)
 
 def p_cpp_class_attribute(s, ctx):
     decorators = None
@@ -3923,14 +3921,6 @@ def p_cpp_class_attribute(s, ctx):
             node.decorators = decorators
         return node
 
-def p_cypclass_lock_mode(s):
-    if s.sy == 'IDENT' and s.systring in ('nolock', 'checklock', 'autolock'):
-        mode = s.systring
-        s.next()
-        return mode
-    else:
-        return None
-
 def p_cypclass_activable(s):
     if s.sy == 'IDENT' and s.systring == 'activable':
         s.next()

Cython/Compiler/PyrexTypes.py

@@ -4243,7 +4243,7 @@ class CppClassType(CType):
             CppClassType(self.name, None, self.cname, [], template_values, template_type=self)\
             if not self.is_cyp_class else\
             CypClassType(self.name, None, self.cname, [], template_values, template_type=self,
-                lock_mode=self.lock_mode, activable=self.activable)
+                activable=self.activable)
         # Need to do these *after* self.specializations[key] is set
         # to avoid infinite recursion on circular references.
         specialized.base_classes = [b.specialize(values) for b in self.base_classes]
@@ -4538,7 +4538,6 @@ def compute_mro_generic(cls):
     return mro_C3_merge(inputs)
 
 class CypClassType(CppClassType):
-    #  lock_mode          string (tri-state: "nolock"/"checklock"/"autolock")
     #  _mro               [CppClassType] or None       the Method Resolution Order of this cypclass according to Python
     #  support_wrapper    boolean                      whether this cypclass will be wrapped
     #  wrapper_type       PyExtensionType or None      the type of the cclass wrapper
@@ -4547,9 +4546,8 @@ class CypClassType(CppClassType):
     to_py_function = None
     from_py_function = None
 
-    def __init__(self, name, scope, cname, base_classes, templates=None, template_type=None, nogil=0, lock_mode=None, activable=False):
+    def __init__(self, name, scope, cname, base_classes, templates=None, template_type=None, nogil=0, activable=False):
         CppClassType.__init__(self, name, scope, cname, base_classes, templates, template_type, nogil)
-        self.lock_mode = lock_mode if lock_mode else "autolock"
         self.activable = activable
         self._mro = None
         self.support_wrapper = False
@@ -4817,6 +4815,7 @@ class QualifiedCypclassType(BaseType):
         'iso': ('iso~',),
         'iso~': (),
         'iso&': ('iso~',),
+        'locked': ('locked', 'iso~'),
     }
 
     def __init__(self, base_type, qualifier):

Cython/Compiler/Symtab.py

@@ -781,7 +781,7 @@ class Scope(object):
 
     def declare_cpp_class(self, name, scope,
             pos, cname = None, base_classes = (),
-            visibility = 'extern', templates = None, cypclass=0, lock_mode=None, activable=False):
+            visibility = 'extern', templates = None, cypclass=0, activable=False):
         if cname is None:
             if self.in_cinclude or (visibility != 'private'):
                 cname = name
@@ -792,7 +792,7 @@ class Scope(object):
         if not entry:
             if cypclass:
                 type = PyrexTypes.CypClassType(
-                    name, scope, cname, base_classes, templates = templates, lock_mode=lock_mode, activable=activable)
+                    name, scope, cname, base_classes, templates = templates, activable = activable)
             else:
                 type = PyrexTypes.CppClassType(
                     name, scope, cname, base_classes, templates = templates)
@@ -3288,6 +3288,8 @@ def qualified_cypclass_scope(base_type_scope, qualifier):
         return ActiveCypclassScope(base_type_scope)
     elif qualifier.startswith('iso'):
         return IsoCypclassScope(base_type_scope)
+    elif qualifier == 'locked':
+        return IsoCypclassScope(base_type_scope, 'locked')
     else:
         return QualifiedCypclassScope(base_type_scope, qualifier)
 

tests/errors/cypclass_lock_error.pyx

-# mode: error
-# tag: cpp, cpp11, pthread
-# cython: experimental_cpp_class_def=True, language_level=2
-
-cdef cypclass A checklock:
-    int a
-    int getter(const self):
-        return self.a
-    void setter(self, int a):
-        self.a = a
-
-cdef void take_write_locked(A obj):
-    pass
-
-cdef int take_read_locked(const A obj):
-    return 3
-
-def incorrect_locks():
-    obj = A()
-    obj.a = 3
-    obj.getter()
-    with rlocked obj:
-        obj.setter(42)
-        take_write_locked(obj)
-    obj.a
-    take_read_locked(obj)
-
-
-cdef A global_cyobject
-
-cdef void global_lock_taking():
-    with wlocked global_cyobject:
-        global_cyobject.setter(global_cyobject.getter() + 1)
-
-_ERRORS = u"""
-20:4: Reference 'obj' is not correctly locked in this expression (write lock required)
-21:4: Reference 'obj' is not correctly locked in this expression (read lock required)
-23:8: Reference 'obj' is not correctly locked in this expression (write lock required)
-24:26: Reference 'obj' is not correctly locked in this expression (write lock required)
-25:4: Reference 'obj' is not correctly locked in this expression (read lock required)
-26:21: Reference 'obj' is not correctly locked in this expression (read lock required)
-32:17: Can only lock local variables or arguments
-"""

tests/run/cypclass_acthon.pyx

@@ -14,7 +14,7 @@ cdef extern from "<semaphore.h>" nogil:
   int sem_destroy(sem_t* sem)
 
 
-cdef cypclass BasicQueue(ActhonQueueInterface) checklock:
+cdef cypclass BasicQueue(ActhonQueueInterface):
   message_queue_t* _queue
 
   __init__(self):
@@ -53,7 +53,7 @@ cdef cypclass BasicQueue(ActhonQueueInterface) checklock:
       # Don't forget to incref to avoid premature deallocation
     return one_message_processed
 
-cdef cypclass NoneResult(ActhonResultInterface) checklock:
+cdef cypclass NoneResult(ActhonResultInterface):
   void pushVoidStarResult(self, void* result):
     pass
   void pushIntResult(self, int result):
@@ -63,7 +63,7 @@ cdef cypclass NoneResult(ActhonResultInterface) checklock:
   int getIntResult(const self):
     return 0
 
-cdef cypclass WaitResult(ActhonResultInterface) checklock:
+cdef cypclass WaitResult(ActhonResultInterface):
   union result_t:
     int int_val
     void* ptr
@@ -104,7 +104,7 @@ cdef cypclass WaitResult(ActhonResultInterface) checklock:
     res = self._getRawResult()
     return res.int_val
 
-cdef cypclass ActivityCounterSync(ActhonSyncInterface) checklock:
+cdef cypclass ActivityCounterSync(ActhonSyncInterface):
   int count
   ActivityCounterSync previous_sync
 
@@ -129,7 +129,7 @@ cdef cypclass ActivityCounterSync(ActhonSyncInterface) checklock:
         res = prev_sync.isCompleted()
     return res
 
-cdef cypclass A checklock activable:
+cdef cypclass A activable:
     int a
     __init__(self):
         self.a = 0
@@ -148,19 +148,17 @@ def test_acthon_chain(n):
     cdef ActhonResultInterface res
     cdef ActhonQueueInterface queue
     sync1 = ActivityCounterSync()
-    with wlocked sync1:
     after_sync1 = ActivityCounterSync(sync1)
+
     obj = A()
-    with wlocked obj:
     obj_actor = activate(obj)
-        with wlocked obj_actor, wlocked sync1, wlocked after_sync1:
+
     # Pushing things in the queue
     obj_actor.setter(sync1, n)
     res = obj_actor.getter(after_sync1)
+
     # Processing the queue
-    with rlocked obj:
     queue = obj._active_queue_class
-    with wlocked queue:
     while not queue.is_empty():
         queue.activate()
     print <int> res

tests/run/cypclass_exception_refcount.pyx

@@ -2,7 +2,7 @@
 # tag: cpp, cpp11
 # cython: experimental_cpp_class_def=True, language_level=2
 
-cdef cypclass Refcounted nolock:
+cdef cypclass Refcounted:
     pass
 
 cdef int raises(Refcounted r) except 0:

tests/run/cypclass_inplace_operator_inference.pyx

@@ -2,7 +2,7 @@
 # tag: cpp, cpp11
 # cython: experimental_cpp_class_def=True, language_level=2
 
-cdef cypclass A nolock:
+cdef cypclass A:
     int val
     __init__(self, int a):
         self.val = a
@@ -10,7 +10,7 @@ cdef cypclass A nolock:
     A __iadd__(self, A other):
         self.val += (other.val + 1)
 
-cdef cypclass B(A) nolock:
+cdef cypclass B(A):
     B __iadd__(self, A other):
         self.val += (other.val + 10)
 

tests/run/cypclass_lock.pyx

-# mode: run
-# tag: cpp, cpp11, pthread
-# cython: experimental_cpp_class_def=True, language_level=2
-
-cdef cypclass A checklock:
-    int a
-    __init__(self):
-        self.a = 0
-    int getter(const self):
-        return self.a
-    void setter(self, int a):
-        self.a = a
-
-def test_basic_locking():
-    """
-    >>> test_basic_locking()
-    0
-    """
-    obj = A()
-    with rlocked obj:
-        print obj.getter()
-
-cdef argument_recursivity(A obj, int arg):
-    if arg > 0:
-        obj.setter(obj.getter() + 1)
-        argument_recursivity(obj, arg - 1)
-
-def test_argument_recursivity(n):
-    """
-    >>> test_argument_recursivity(42)
-    42
-    """
-    obj = A()
-    with wlocked obj:
-        argument_recursivity(obj, n)
-        print obj.a
-
-cdef cypclass Container:
-    A object
-    __init__(self):
-        self.object = A()
-
-def test_lock_traversal(n):
-    """
-    >>> test_lock_traversal(42)
-    42
-    """
-    container = Container()
-    with rlocked container:
-        contained = container.object
-        with wlocked contained:
-            argument_recursivity(contained, n)
-            print contained.getter()

tests/run/cypclass_nested_classes.pyx

@@ -2,10 +2,10 @@
 # tag: cpp, cpp11
 # cython: experimental_cpp_class_def=True, language_level=2
 
-cdef cypclass A nolock:
+cdef cypclass A:
     int a
 
-    cypclass B nolock:
+    cypclass B:
         int b
         __init__(self, int b):
             self.b = b