Skip to content

Fix thread-unsafe init cache rebuild and unmask errors - #1171

Merged
philippjfr merged 3 commits into
mainfrom
fix_thread_unsafe_cache
Aug 24, 2026
Merged

Fix thread-unsafe init cache rebuild and unmask errors#1171
philippjfr merged 3 commits into
mainfrom
fix_thread_unsafe_cache

Conversation

@philippjfr

Copy link
Copy Markdown
Member

Problem

Concurrent code that temporarily toggles Parameter slots (the pattern Panel uses in
panel.util.parameters.edit_readonly) can crash with a completely misleading error:

File "panel/io/state.py", line 428, in _cleanup_busy_counter
    with edit_readonly(self):
File "panel/util/parameters.py", line 47, in edit_readonly
    params = parameterized.param.objects("existing").values()
File "param/parameterized.py", line 3238, in objects
    pdict = self_._cls_parameters
File "param/parameterized.py", line 2570, in __getattr__
    raise AttributeError(f"'{self_.cls.__name__}.param' object has no attribute {attr!r}")
AttributeError: '_state.param' object has no attribute '_cls_parameters'. Did you mean: 'add_parameter'?

_cls_parameters obviously does exist, which makes this very hard to diagnose.

Root cause

Two independent issues compound each other.

1. The init cache rebuild is not thread safe. Assigning constant, instantiate or
default_factory on a Parameter calls Parameter._invalidate_init_cache(), which resets
params_to_deepcopy, params_to_ref and params_with_default_factory on the owner's
_ClassPrivate to None. The cached-parameters branch of _cls_parameters rebuilt those
lists by assigning [] to each attribute and then appending to them through the attribute
while iterating the parameters. If another thread invalidated the caches part way through
that loop, the next private.params_to_ref.append(pobj) hit None and raised
AttributeError: 'NoneType' object has no attribute 'append'.

2. Parameters.__getattr__ masked the real error. _cls_parameters is a property, and
an AttributeError raised inside a property getter is indistinguishable from a missing
attribute: CPython clears it and dispatches to __getattr__. Parameters.__getattr__ then
evaluated attr in self_._cls_parameters, which on this retry succeeded (the three cache
attributes had already been reassigned by the failed call), found that '_cls_parameters'
is not a parameter name, and raised the bogus "has no attribute" error. The real exception
and its traceback were gone, with no __context__ to follow. Had the retry failed too, the
result would have been a RecursionError instead.

Panel triggers this because _state is a process-wide singleton whose busy counter is
updated from multiple threads, and every edit_readonly(state) enter and exit flips
constant on every one of its parameters.

Changes

Parameter.__setattr__ no longer invalidates the init caches when one of the cache
attributes is set to the value it already has. The caches are derived purely from those
attributes, so a no-op set cannot change them. This removes the invalidation entirely for
the save/restore pattern above, which also avoids repeatedly rebuilding the caches for
every parameter of the class.

Parameters._cls_parameters now builds the three lists in locals and only publishes them
once complete, matching what the cold path already did. A concurrent invalidation can no
longer be observed mid-rebuild.

Parameters.__getattr__ no longer masks errors raised by this class' own descriptors. It
reads the cached class parameters directly from _param__private.params rather than going
through the _cls_parameters property, so it cannot recurse into the property that sent it
there, and it falls back to re-invoking the descriptor via the new _invoke_descriptor
helper when the requested attribute is not a parameter but does exist on the class. The
real error then propagates with its own traceback.

Notes

Reading _param__private.params directly in __getattr__ also skips the derived cache
checks that the property performs but that __getattr__ does not need, so obj.param.<name>
access gets slightly faster (roughly 490ns to 450ns per access locally).

Tests

Four tests in tests/testparameterizedobject.py, all failing before this change:

  • test_no_op_slot_set_does_not_invalidate_init_cache
  • test_cls_parameters_rebuild_survives_concurrent_invalidation, a deterministic
    simulation of an invalidation landing in the middle of the rebuild loop
  • test_cls_parameters_rebuild_is_thread_safe, four threads running the edit_readonly
    save/flip/restore pattern, which reproduced the original error in 6 of 6 runs before the
    fix
  • test_param_namespace_getattr_does_not_mask_descriptor_errors

Verified end to end against Panel as well: eight threads hammering
state._add_busy_event/_remove_busy_event reproduce the reported _cls_parameters error
within seconds on main and run clean with this change.

Note that edit_readonly remains unsafe under concurrency for a separate reason (two
overlapping calls can snapshot each other's temporarily relaxed readonly/constant
values and restore the wrong ones). That needs fixing on the Panel side.

AI Disclosure

Fix developed with assistance from Claude Opus 5

@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.83333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.75%. Comparing base (bd6b7f9) to head (1bc2219).

Files with missing lines Patch % Lines
param/parameterized.py 95.83% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1171      +/-   ##
==========================================
+ Coverage   86.73%   86.75%   +0.01%     
==========================================
  Files           9        9              
  Lines        5321     5336      +15     
==========================================
+ Hits         4615     4629      +14     
- Misses        706      707       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread param/parameterized.py Outdated
Comment thread param/parameterized.py Outdated
Comment thread param/parameterized.py Outdated
Comment thread tests/testparameterizedobject.py Outdated
@philippjfr
philippjfr merged commit b2f6f85 into main Aug 24, 2026
18 checks passed
@philippjfr
philippjfr deleted the fix_thread_unsafe_cache branch August 24, 2026 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants