From 5ebe3a2d841f8f69d0d4214be3e88859a4a29f80 Mon Sep 17 00:00:00 2001 From: Harry Brady Date: Tue, 4 Aug 2026 09:03:11 +0100 Subject: [PATCH 1/2] fix: make lazy graph object properties thread-safe AI-Authored-By: openai/gpt-6-astra --- CHANGELOG.md | 3 + plotly/basedatatypes.py | 15 ++-- .../test_graph_objs/test_thread_safety.py | 89 +++++++++++++++++++ 3 files changed, 101 insertions(+), 6 deletions(-) create mode 100644 tests/test_core/test_graph_objs/test_thread_safety.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 65abbd3d003..0f4c6653787 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased +### Fixed +- Fix concurrent first access to lazily initialized graph object properties, which could raise `ValueError("Invalid value")` [[#3441](https://github.com/plotly/plotly.py/issues/3441)] + ## [7.1.0] - 2026-09-15 diff --git a/plotly/basedatatypes.py b/plotly/basedatatypes.py index d81ba0950c1..a7326da8e5a 100644 --- a/plotly/basedatatypes.py +++ b/plotly/basedatatypes.py @@ -4694,24 +4694,27 @@ def __getitem__(self, prop): if isinstance(validator, CompoundValidator): if self._compound_props.get(prop, None) is None: # Init compound objects - self._compound_props[prop] = validator.data_class( - _parent=self, plotly_name=prop - ) + child = validator.data_class(_parent=self, plotly_name=prop) # Update plotly_name value in case the validator applies # non-standard name (e.g. imagedefaults instead of image) - self._compound_props[prop]._plotly_name = prop + child._plotly_name = prop + # Concurrent readers may both construct a child, but they + # must use the first child published to the cache. + self._compound_props.setdefault(prop, child) return validator.present(self._compound_props[prop]) elif isinstance(validator, (CompoundArrayValidator, BaseDataValidator)): if self._compound_array_props.get(prop, None) is None: # Init list of compound objects if self._props is not None: - self._compound_array_props[prop] = [ + children = [ validator.data_class(_parent=self) for _ in self._props.get(prop, []) ] else: - self._compound_array_props[prop] = [] + children = [] + # Keep every concurrent reader attached to the same list. + self._compound_array_props.setdefault(prop, children) return validator.present(self._compound_array_props[prop]) elif self._props is not None and prop in self._props: diff --git a/tests/test_core/test_graph_objs/test_thread_safety.py b/tests/test_core/test_graph_objs/test_thread_safety.py new file mode 100644 index 00000000000..c84f4499b8f --- /dev/null +++ b/tests/test_core/test_graph_objs/test_thread_safety.py @@ -0,0 +1,89 @@ +import threading + +import pytest + +import plotly.graph_objs as go + + +@pytest.mark.parametrize( + ("target_type", "target_kwargs", "property_name", "child_property", "expected"), + [ + pytest.param( + go.Layout, + {"font": {"family": "Arial"}}, + "font", + "family", + "Arial", + id="compound-property", + ), + pytest.param( + go.layout.template.Data, + {"bar": [{"name": "template bar"}]}, + "bar", + "name", + "template bar", + id="compound-array-property", + ), + ], +) +def test_concurrent_first_read_keeps_children_attached( + monkeypatch, + target_type, + target_kwargs, + property_name, + child_property, + expected, +): + target = target_type(**target_kwargs) + target._compound_props.pop(property_name, None) + target._compound_array_props.pop(property_name, None) + validator = target._get_validator(property_name) + data_class = validator.data_class + constructors_ready = threading.Barrier(2) + first_read_complete = threading.Event() + second_read_complete = threading.Event() + children = [] + results = [] + errors = [] + + def build_child(*args, **kwargs): + child = data_class(*args, **kwargs) + constructors_ready.wait(timeout=5) + if threading.current_thread().name == "second-reader": + if not first_read_complete.wait(timeout=5): + raise TimeoutError("First reader did not receive its child") + return child + + monkeypatch.setattr(validator, "_data_class", build_child) + + def read_child(): + try: + value = target[property_name] + if threading.current_thread().name == "first-reader": + first_read_complete.set() + if not second_read_complete.wait(timeout=5): + raise TimeoutError("Second reader did not receive its child") + else: + second_read_complete.set() + + child = value[0] if isinstance(value, tuple) else value + children.append(child) + results.append(child[child_property]) + except Exception as error: + errors.append(error) + first_read_complete.set() + second_read_complete.set() + + workers = [ + threading.Thread(target=read_child, name="first-reader"), + threading.Thread(target=read_child, name="second-reader"), + ] + for worker in workers: + worker.start() + for worker in workers: + worker.join(timeout=5) + + assert all(not worker.is_alive() for worker in workers) + assert not errors + assert children[0] is children[1] + assert results == [expected, expected] From 1ba2769f5cb013cc6f6de1e2cc65f7a35e3de793 Mon Sep 17 00:00:00 2001 From: Harry Brady <39841275+hb1915@users.noreply.github.com> Date: Thu, 17 Sep 2026 09:45:41 +0100 Subject: [PATCH 2/2] Apply suggestion from @KoolADE85 Co-authored-by: Adrian Borrmann --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f4c6653787..7107261c9f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased ### Fixed -- Fix concurrent first access to lazily initialized graph object properties, which could raise `ValueError("Invalid value")` [[#3441](https://github.com/plotly/plotly.py/issues/3441)] +- Fix concurrent first access to lazily initialized graph object properties, which could raise `ValueError("Invalid value")` [[#3441](https://github.com/plotly/plotly.py/issues/3441)], with thanks to @hb1915 for the contribution! ## [7.1.0] - 2026-09-15