Skip to content

Commit cfc2943

Browse files
dcjclaude
andcommitted
homie: snapshot _nodes/_children before iteration to fix MQTT-thread race (SDK-e3k)
publish_nodes() and refresh_tree() were iterating self._nodes / self._children live while the main thread mutated them via add_node() / Device(parent=...) construction. On a SPAN G2 panel this crashed the MQTT loop thread on initial broker connect: File ".../ebus_sdk/homie.py", line 1501, in publish_nodes for node in self._nodes.values(): RuntimeError: dictionary changed size during iteration systemd then SIGKILLed the unresponsive process; the restart recovered. Both call sites now iterate through a list() snapshot, matching the defensive pattern already in place in delete_all_from_mqtt() (line 1246) and delete() (line 1305). Adds two deterministic regression tests that fail on the unfixed code and pass with the snapshot. refresh_tree() doesn't actually crash without the fix (Python lists don't raise on mutation-during-iteration the way dicts do), but CPython's list iterator would pull a half-constructed child appended mid-cascade into the current reconnect republish — so the snapshot is the correct semantics either way. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 586a390 commit cfc2943

5 files changed

Lines changed: 65 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ All notable changes to `ebus-sdk` are recorded here. Format follows [Keep a Chan
44

55
## [Unreleased]
66

7+
## [0.2.2] — 2026-06-12
8+
9+
### Fixed
10+
11+
- `Device.publish_nodes()` no longer crashes with `RuntimeError: dictionary changed size during iteration` when the main thread adds a node while the MQTT loop thread is publishing on initial broker connect. Iteration now goes through a `list(self._nodes.values())` snapshot — matching the defensive pattern already used in `delete_all_from_mqtt()`. Hit on a SPAN G2 panel immediately after deploy; systemd subsequently SIGKILLed the unresponsive process, restart recovered.
12+
- `Device.refresh_tree()` similarly snapshots `self._children` before recursing. Lists don't raise on mutation-during-iteration, but CPython's list iterator would otherwise pull a half-constructed child added mid-cascade into the current reconnect republish — matching the defensive pattern in `Device.delete()`.
13+
714
## [0.2.1] — 2026-06-12
815

916
### Added
@@ -53,7 +60,8 @@ The 0.2.0 release introduces first-class parent/child device trees on both the d
5360

5461
Initial public release on PyPI. See `git log v0.1.2` for the surface that shipped.
5562

56-
[Unreleased]: https://github.com/electrification-bus/python-sdk/compare/v0.2.1...HEAD
63+
[Unreleased]: https://github.com/electrification-bus/python-sdk/compare/v0.2.2...HEAD
64+
[0.2.2]: https://github.com/electrification-bus/python-sdk/releases/tag/v0.2.2
5765
[0.2.1]: https://github.com/electrification-bus/python-sdk/releases/tag/v0.2.1
5866
[0.2.0]: https://github.com/electrification-bus/python-sdk/releases/tag/v0.2.0
5967
[0.1.7]: https://github.com/electrification-bus/python-sdk/releases/tag/v0.1.7

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
44

55
[project]
66
name = "ebus-sdk"
7-
version = "0.2.1"
7+
version = "0.2.2"
88
description = "Python SDK for Homie MQTT Convention (eBus)"
99
readme = "README.md"
1010
requires-python = ">=3.10"

src/ebus_sdk/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
# MQTT client
4444
from ebus_mqtt_client import MqttClient
4545

46-
__version__ = "0.2.1"
46+
__version__ = "0.2.2"
4747

4848
__all__ = [
4949
# Homie classes

src/ebus_sdk/homie.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1421,7 +1421,9 @@ def refresh_tree(self) -> None:
14211421
f"nodeCount={len(self._nodes)},childCount={len(self._children)}"
14221422
)
14231423
self._publish_self()
1424-
for child in self._children:
1424+
# Snapshot — main thread may construct child devices (which append
1425+
# to self._children) while this runs on the MQTT loop thread.
1426+
for child in list(self._children):
14251427
child.refresh_tree()
14261428

14271429
def publish(self, attribute: str = "", value: Optional[Any] = None) -> None:
@@ -1498,7 +1500,11 @@ def publish_description(self, republish: bool = False) -> None:
14981500
self.publish("$description")
14991501

15001502
def publish_nodes(self) -> None:
1501-
for node in self._nodes.values():
1503+
# Snapshot — invoked from on_connect() on the MQTT loop thread while
1504+
# the main thread may be inside state_transition() calling add_node().
1505+
# Without the snapshot, dict-size-changed-during-iteration crashes the
1506+
# MQTT thread on initial connect.
1507+
for node in list(self._nodes.values()):
15021508
node.publish()
15031509

15041510
def on_connect(self) -> None:

tests/test_homie_device.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,6 +1224,25 @@ def test_publish_nodes(self, mock_paho):
12241224
device.publish_nodes()
12251225
mock_node.publish.assert_called_once()
12261226

1227+
def test_publish_nodes_snapshots_against_concurrent_add(self, mock_paho):
1228+
"""SDK-e3k: publish_nodes() must snapshot self._nodes so the main
1229+
thread adding a node mid-iteration doesn't raise
1230+
'dictionary changed size during iteration' on the MQTT loop thread."""
1231+
device, _ = _make_device(mock_paho)
1232+
1233+
# Simulate the race: while iterating, one node's publish() mutates
1234+
# the underlying dict (as the main thread's add_node would).
1235+
racing_node = MagicMock()
1236+
1237+
def mutate_during_publish():
1238+
device._nodes["late-arrival"] = MagicMock()
1239+
1240+
racing_node.publish.side_effect = mutate_during_publish
1241+
device._nodes = {"core": racing_node}
1242+
1243+
# Without the list() snapshot fix, this raises RuntimeError.
1244+
device.publish_nodes()
1245+
12271246

12281247
class TestDeviceOnConnect:
12291248
def test_initial_connection(self, mock_paho):
@@ -1358,6 +1377,33 @@ def test_refresh_tree_cascades_to_children(self, mock_paho):
13581377
)
13591378
assert any(f"/{device_id}/$state" in t for t in topics), f"missing $state for {device_id} in {topics}"
13601379

1380+
def test_refresh_tree_snapshots_against_concurrent_child_add(self, mock_paho):
1381+
"""SDK-e3k: refresh_tree() must snapshot self._children so a child
1382+
appended by the main thread mid-cascade isn't pulled into the current
1383+
republish on the MQTT loop thread. (Lists don't raise on
1384+
mutation-during-iteration the way dicts do, but processing a
1385+
half-constructed child is its own correctness hazard.)"""
1386+
root, _ = _make_device(mock_paho, device_id="panel-1")
1387+
existing_child = Device(id="circuit-a", parent=root)
1388+
late_arrival = MagicMock(spec=Device)
1389+
1390+
original_refresh = existing_child.refresh_tree
1391+
1392+
def mutate_during_refresh():
1393+
# Simulate the main thread appending a new child while the MQTT
1394+
# thread is mid-cascade.
1395+
root._children.append(late_arrival)
1396+
original_refresh()
1397+
1398+
existing_child.refresh_tree = mutate_during_refresh
1399+
1400+
root.refresh_tree()
1401+
1402+
# Snapshot semantics: late_arrival was appended after iteration began,
1403+
# so it must NOT be touched by this refresh cycle. Without the
1404+
# list() snapshot, CPython's list iterator picks it up.
1405+
late_arrival.refresh_tree.assert_not_called()
1406+
13611407
def test_refresh_tree_three_levels(self, mock_paho):
13621408
"""S2 + S6: grandchildren also republish on reconnect."""
13631409
root, mock_client = _make_device(mock_paho, device_id="panel-1")

0 commit comments

Comments
 (0)