Skip to content

Commit 3a863e3

Browse files
authored
fix: two thread-safety fixes (0.20.1) - property publish lock, per-thread bulk context (#56)
Two thread-safety fixes, both in the model layer. Property (#50 follow-up): compute-payload/publish/memoize is now one atomic unit per property under a reentrant lock. Interleaved with a reconnect refresh, the 0.20.0 memo could record an older payload than the one last written, after which the publish-on-change gate suppressed the correcting publish and the broker kept a wrong retained value. GroupedPropertyDict (#55): the active bulk-update context is now thread-local. Two threads entering bulk contexts on one dict displaced each other and the first to exit ended bulk mode for both, fragmenting the survivor batch into individual events (extra $description republishes and $state flapping downstream). Also: CI jobs that run code gain timeout-minutes, so a lock regression fails instead of running to GitHub 6-hour default.
1 parent 67300e0 commit 3a863e3

9 files changed

Lines changed: 372 additions & 124 deletions

File tree

.github/workflows/lint.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ on:
88
jobs:
99
ruff:
1010
runs-on: ubuntu-latest
11+
timeout-minutes: 5
1112
steps:
1213
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
1314
- uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0

.github/workflows/publish.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ permissions:
1212
jobs:
1313
test:
1414
runs-on: ubuntu-latest
15+
# The publish gate: a hang here must fail, not stall the release.
16+
timeout-minutes: 5
1517
steps:
1618
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
1719
- uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0

.github/workflows/test.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ jobs:
99
pytest:
1010
name: pytest (py${{ matrix.python-version }})
1111
runs-on: ubuntu-latest
12+
# The suite runs in ~2s, so anything approaching this is a hang, not slowness.
13+
# Since 0.20.1 homie.Property takes a lock, and a lock regression (notably making
14+
# it non-reentrant) deadlocks the thread that hits it: without a bound, GitHub
15+
# would let that run to the 6-hour default rather than reporting a failure.
16+
timeout-minutes: 5
1217
strategy:
1318
# Run every version even if one fails, so a single-version break is
1419
# visible as exactly that rather than masking the rest.

CHANGELOG.md

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

55
## [Unreleased]
66

7+
## [0.20.1] — 2026-08-13
8+
9+
### Changed
10+
11+
- CI: the `pytest` and `ruff` jobs, and the publish workflow's test gate, now carry `timeout-minutes: 5`. The suite runs in about two seconds, so anything near that bound is a hang rather than slowness. This matters from this release on: `homie.Property` now takes a lock, and a lock regression deadlocks whichever thread reaches it (making it non-reentrant deadlocks the main thread at the first `set_value`, which is most of the suite). Unbounded, GitHub would let that run to its six-hour default instead of reporting a failure, and a hung job reports nothing useful. The publish and release jobs are deliberately left unbounded, since a slow PyPI upload is not the same kind of event.
12+
13+
### Fixed
14+
15+
- `GroupedPropertyDict`'s active bulk-update context is now per-thread. `BulkUpdateContext.__enter__` and `__exit__` mutated a shared `_bulk_mode`/`_bulk_context` pair with no lock held, while every other accessor on the class (including the observer dispatch) took its `RLock`, which made the omission look accidental rather than deliberate. Two threads entering bulk contexts on the same dict therefore corrupted each other: entering displaced the other's context, and whichever exited first cleared bulk mode for both. No events were lost, since `__exit__` fires the context object's own list, but they were misattributed and fragmented: some of one thread's changes landed in the other's batch, and everything after the early exit fired individually instead of batching. For a Homie publisher that fragmentation is the real cost, because each structural event that escapes the batch triggers its own `$description` republish and a `$state` transition, so one logical change produces extra republishes and visible state flapping. Thread-local rather than serializing on the existing lock, since a bulk context can be held across I/O and one thread should not block for the duration of another's batch. A nested `bulk_update()` on the same thread now restores the enclosing context on exit instead of ending it, so the outer batch resumes rather than leaking its remainder as individual events. Two threads batching independently was always the reasonable reading; now it is the actual behavior. Reported with a precise account of which consequences do and do not follow. ([#55](https://github.com/electrification-bus/python-sdk/issues/55))
16+
17+
- `Property` now serializes "compute the payload, publish it, record what was published" under a per-property reentrant lock, so the publish-on-change memo can never disagree with the last write that actually reached the wire. Two threads reach that sequence: the application thread via `set_value()`, and the MQTT loop thread via `on_connect` → `refresh_tree(force=True)`. Interleaved, the loop thread could publish the old payload, the application thread could then publish and memoize the new one, and the loop thread could finally overwrite the memo with the older payload it had sent first. The property then believed the broker held a value it did not, and the 0.20.0 gate suppressed the very publish that would have corrected it, so the wrong retained value persisted until the next genuine change or reconnect. The window is narrow (it needs a reconnect refresh concurrent with a value update) and the class has never had a lock, so `_value` and `_ever_published` were already exposed to it in kind; 0.20.0 made the consequence durable rather than transient, which is what moves this from a latent wart to a fix. The lock is reentrant because `set_value()` calls `publish_value()` calls `clear_value()`, each taking it; a plain `Lock` self-deadlocks on the commonest call in the SDK. It is per-property, so it never serializes a tree walk, and no path holds two, so there is no ordering hazard. It is deliberately held across the transport's `publish()`: releasing earlier reopens the window it exists to close. No API change. ([#50](https://github.com/electrification-bus/python-sdk/issues/50))
18+
719
## [0.20.0] — 2026-08-12
820

921
### Added
@@ -314,7 +326,8 @@ The 0.2.0 release introduces first-class parent/child device trees on both the d
314326

315327
Initial public release on PyPI. It predates this repo's tagging convention (the earliest tag is `v0.1.4`), so there is no `v0.1.2` tag to read; the published artifact on PyPI is the record of the surface that shipped.
316328

317-
[Unreleased]: https://github.com/electrification-bus/python-sdk/compare/v0.20.0...HEAD
329+
[Unreleased]: https://github.com/electrification-bus/python-sdk/compare/v0.20.1...HEAD
330+
[0.20.1]: https://github.com/electrification-bus/python-sdk/releases/tag/v0.20.1
318331
[0.20.0]: https://github.com/electrification-bus/python-sdk/releases/tag/v0.20.0
319332
[0.19.0]: https://github.com/electrification-bus/python-sdk/releases/tag/v0.19.0
320333
[0.18.1]: https://github.com/electrification-bus/python-sdk/releases/tag/v0.18.1

src/ebus_sdk/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@
8181
# Structural types for a caller-supplied MQTT client
8282
from ebus_sdk.transport import MqttControllerTransport, MqttDeviceTransport, MqttTransport
8383

84-
__version__ = "0.20.0"
84+
__version__ = "0.20.1"
8585

8686
__all__ = [
8787
# Homie classes

0 commit comments

Comments
 (0)