Fix thread-unsafe init cache rebuild and unmask errors

param/parameterized.py

@@ -1855,7 +1855,7 @@ def __setattr__(self, attribute: str, value):
                 pass
 
         super().__setattr__(attribute, value)
-        if is_slot and attribute in _PARAMETER_CACHE_ATTRS:
+        if is_slot and attribute in _PARAMETER_CACHE_ATTRS and value is not old:
             self._invalidate_init_cache()
         if has_watcher and old is not NotImplemented:
             self._trigger_event(attribute, old, value)
@@ -2411,6 +2411,21 @@ def __exit__(self, exc_type, exc_value, exc_tb):
             self._restore = {}
 
 
+def _find_descriptor(cls: type, attr: str) -> t.Any:
+    """
+    Return the descriptor implementing ``attr`` on ``cls``, or None.
+
+    Used to invoke a descriptor without going through attribute lookup on the
+    object, since an AttributeError raised inside a descriptor is
+    indistinguishable from a missing attribute to the attribute machinery,
+    which clears it and dispatches to ``__getattr__``.
+    """
+    for klass in cls.__mro__:
+        if attr in klass.__dict__:
+            return klass.__dict__[attr]
+    return None
+
+
 class Parameters:
     """
     Object that holds the ``.param`` namespace and implementation of
@@ -2562,9 +2577,26 @@ def __getattr__(self_, attr: str) -> t.Any:
         if cls is None: # Class not initialized
             raise AttributeError
 
-        if attr in self_._cls_parameters:
+        ns_type = type(self_)
+
+        # The cached parameters are read from _param__private directly rather
+        # than via the _cls_parameters property. If that property raised an
+        # AttributeError we would be called to handle it and recurse, so it is
+        # only invoked as a descriptor, i.e. without attribute lookup on self_.
+        params = cls._param__private.params
+        if not params:
+            params = _find_descriptor(ns_type, '_cls_parameters').__get__(self_, ns_type)
+        if attr in params:
             return self_.__getitem__(attr)
-        elif self_.self is None:
+
+        # attr is not a Parameter, so if it does exist on this class the
+        # AttributeError we are handling was raised inside its descriptor.
+        # Invoking it again surfaces that error instead of masking it.
+        descriptor = _find_descriptor(ns_type, attr)
+        if descriptor is not None:
+            return descriptor.__get__(self_, ns_type)
+
+        if self_.self is None:
             raise AttributeError(f"type object '{self_.cls.__name__}.param' has no attribute {attr!r}")
         else:
             raise AttributeError(f"'{self_.cls.__name__}.param' object has no attribute {attr!r}")
@@ -3140,18 +3172,21 @@ def _cls_parameters(self_) -> dict[str, Parameter]:
         pdict = private.params
         if pdict:
             if private.params_to_deepcopy is None or private.params_to_ref is None or private.params_with_default_factory is None:
-                private.params_to_deepcopy = []
-                private.params_to_ref = []
-                private.params_with_default_factory = []
+                to_deepcopy = []
+                to_ref = []
+                with_default_factory = []
                 for pname, pobj in pdict.items():
                     if pname == 'name':
                         continue
                     if pobj.default_factory is not None:
-                        private.params_with_default_factory.append((pname, pobj))
+                        with_default_factory.append((pname, pobj))
                     elif pobj.instantiate:
-                        private.params_to_deepcopy.append(pobj)
+                        to_deepcopy.append(pobj)
                     elif pobj.constant:
-                        private.params_to_ref.append(pobj)
+                        to_ref.append(pobj)
+                private.params_to_deepcopy = to_deepcopy
+                private.params_to_ref = to_ref
+                private.params_with_default_factory = with_default_factory
             return pdict
 
         paramdict = {}

tests/testparameterizedobject.py

@@ -3,6 +3,7 @@
 import inspect
 import re
 import sys
+import threading
 import unittest
 import warnings
 import weakref
@@ -2002,3 +2003,108 @@ class P(param.Parameterized):
 
     del obj
     assert freed, "Parameterized instance not freed immediately — likely a reference cycle via .param"
+
+
+def test_no_op_slot_set_does_not_invalidate_init_cache():
+    class P(param.Parameterized):
+        x = param.Number(1)
+
+    P.param.objects('existing')
+    private = P._param__private
+    assert private.params_to_deepcopy is not None
+
+    P.param.x.constant = P.param.x.constant
+    P.param.x.instantiate = P.param.x.instantiate
+    assert private.params_to_deepcopy is not None
+
+    P.param.x.constant = not P.param.x.constant
+    assert private.params_to_deepcopy is None
+
+
+def test_cls_parameters_rebuild_survives_concurrent_invalidation():
+    # Setting constant/instantiate/default_factory on a Parameter invalidates
+    # the init caches. If that happens while _cls_parameters is rebuilding
+    # them, the rebuild must not fail (it used to append to None).
+    class Invalidating(param.Parameter):
+
+        def __getattribute__(self, key):
+            if key == 'instantiate':
+                try:
+                    owner = object.__getattribute__(self, 'owner')
+                except AttributeError:
+                    owner = None
+                if owner is not None:
+                    private = owner._param__private
+                    private.params_to_deepcopy = None
+                    private.params_to_ref = None
+                    private.params_with_default_factory = None
+            return super().__getattribute__(key)
+
+    class P(param.Parameterized):
+        a = Invalidating()
+        b = param.String(constant=True)
+
+    assert set(P.param.objects('existing')) == {'name', 'a', 'b'}
+
+    # Only invalidate the init caches, so that the rebuild happens in the
+    # branch that reuses the already cached parameters
+    private = P._param__private
+    assert private.params
+    private.params_to_deepcopy = None
+    private.params_to_ref = None
+    private.params_with_default_factory = None
+
+    assert set(P.param.objects('existing')) == {'name', 'a', 'b'}
+
+
+def test_cls_parameters_rebuild_is_thread_safe():
+    class P(param.Parameterized):
+        pass
+
+    for i in range(100):
+        P.param.add_parameter(f'p{i}', param.Integer(default=i, constant=bool(i % 2)))
+
+    p = P()
+    errors = []
+    stop = threading.Event()
+
+    def toggle_readonly():
+        # Mimics panel.util.parameters.edit_readonly
+        try:
+            for _ in range(500):
+                if stop.is_set():
+                    return
+                params = list(p.param.objects('existing').values())
+                constants = [po.constant for po in params]
+                for po in params:
+                    po.constant = False
+                for po, constant in zip(params, constants):
+                    po.constant = constant
+        except Exception as e:
+            errors.append(e)
+            stop.set()
+
+    threads = [threading.Thread(target=toggle_readonly) for _ in range(4)]
+    for t in threads:
+        t.start()
+    for t in threads:
+        t.join()
+
+    assert not errors, errors[0]
+
+
+def test_param_namespace_getattr_does_not_mask_descriptor_errors():
+    class P(param.Parameterized):
+        x = param.Number(1)
+
+    original = parameterized.Parameters._cls_parameters
+
+    def broken(self_):
+        raise AttributeError('the real error')
+
+    try:
+        parameterized.Parameters._cls_parameters = property(broken)
+        with pytest.raises(AttributeError, match='the real error'):
+            P().param.objects('existing')
+    finally:
+        parameterized.Parameters._cls_parameters = original