fix: two thread-safety fixes (0.20.1) — property publish lock, per-thread bulk context

[dcj]

Closes the race flagged as a known limitation in #51, rather than carrying it. Also bumps to 0.20.1.

The defect

0.20.0's publish-on-change memo could record an older payload than the one that actually reached the wire last. The gate then suppressed the very publish that would have corrected the broker.

Two threads reach compute-payload/publish/memoize:

• the application thread, via set_value()
• the MQTT loop thread, via on_connect → refresh_tree(force=True) → Node.publish() → publish_value(force=True)

Interleaved: the loop thread sends the old payload; the application thread then sends and memoizes the new one; the loop thread finally overwrites the memo with the older payload it sent first. The property now believes the broker holds a value it does not, and a later set_value() of that value is skipped. The wrong retained value persists until the next genuine change or reconnect.

The window is narrow (it needs a reconnect refresh concurrent with a value update), and homie.Property has never had a lock, so _value and _ever_published were already exposed to it in kind. What 0.20.0 changed is that the consequence became durable rather than transient. That is what moves it from a latent wart to a fix.

The fix

A per-property RLock, held across compute/publish/memoize in set_value(), publish_value(), clear_value() and invalidate_publish_cache().

Reentrant, and not optionally so. set_value() takes it and calls publish_value(), which on the retraction path calls clear_value(). A plain Lock self-deadlocks on the commonest call in the SDK — verified by mutation, and the reason is recorded in the code so nobody "simplifies" it back.

Held across the transport's publish(), deliberately. Releasing earlier reopens the window it exists to close. That is safe for the paho transport the SDK ships, and the code records why, because it is not obvious. The dangerous shape would be an A-B/B-A cycle where the network thread holds a transport lock while invoking on_connect (→ refresh_tree → publish_value, which wants this lock) that publish() also needs. In paho 2.x it does not arise: _handle_connack invokes on_connect holding only _in_callback_mutex and acquires _out_message_mutex only after the callback returns (the two blocks are sequential, not nested), and the one place the publish path touches _in_callback_mutex (_packet_queue) uses a non-blocking acquire(False) that threaded mode skips entirely.

A bring-your-own transport could still construct that cycle by holding its own lock across the on-connect handler it wires to refresh_tree() while also requiring that lock in publish(). The code says so.

Per-property, so it never serializes a tree walk, and no path holds two, so there is no lock-ordering hazard.

Also

• get_last_published_value() reads the memo tuple once into a local instead of testing the attribute and then subscripting it. Under the GIL that window is practically unreachable; a free-threaded build removes the accident, and CI covers 3.13.
• CI jobs that run code gain timeout-minutes: 5. The suite runs in ~2s. 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 — and unbounded, GitHub would let that run to its six-hour default instead of reporting a failure. Publish/release jobs are left unbounded: a slow PyPI upload is not the same event.

Verification

• 598 tests (596 before), ruff check + ruff format --check clean, markdownlint clean.
• Two new tests in TestHomiePropertyPublishThreadSafety, both mutation-verified:
  • removing the lock (RLock() → nullcontext()) fails test_a_concurrent_forced_republish_cannot_leave_a_stale_memo, reproducing the race exactly: memo 1.0 while the wire last carried 2.0.
  • RLock → Lock fails test_retraction_through_the_lock_does_not_deadlock.
• Threaded tests run 20/20 clean with no flakiness.

Note most of the homie.py diff is reindentation of two method bodies under a with block; git diff -w shows the real change is ~57 lines.

An adversarial review raised 8 findings and 7 were refuted (several TOCTOU claims were disproven by measurement). The one that survived — worker threads in the new test being non-daemon, so a future deadlock regression would hang CI after reporting the failure — is fixed with daemon=True, and chasing it is what surfaced the missing CI timeouts above.

🤖 Generated with Claude Code

[dcj]

---

Update: also closes #55 (filed while this PR was open)

Same class of defect, one layer down, so it rides along in 0.20.1 rather than waiting.

BulkUpdateContext.__enter__/__exit__ mutated a shared _bulk_mode/_bulk_context pair on GroupedPropertyDict with no lock held, while every other accessor on the class (including the observer dispatch and the group scans) takes its RLock.

Two threads entering bulk contexts on the same dict corrupted each other: entering displaced the other's context, and whichever exited first cleared bulk mode for both. Per #55's own analysis, and it is worth keeping the distinction: no events are lost (__exit__ fires the context object's own list), but they are misattributed and fragmented. Some of one thread's changes land in the other's batch, and everything after the early exit fires individually instead of batching.

The fragmentation is what actually hurts a Homie publisher: each structural event escaping the batch triggers its own $description republish and a $state transition, so one logical change produces extra republishes and visible state flapping.

Fixed thread-local, which is the option #55 recommends and for the reason it gives: a bulk context can be held across I/O, and serializing on the existing lock would make one thread block for the duration of another's batch. Two threads batching independently was always the reasonable reading of this API.

Nesting improves as a side effect. A context now restores the enclosing one on exit instead of clearing it, so a nested bulk_update() on the same thread lets the outer batch resume rather than leaking its remainder as individual events.

Verification

• 600 tests (598 after the lock fix, 596 before this PR).
• test_concurrent_bulk_updates_do_not_fragment_each_others_batches drives the exact two-thread interleaving (B opens and closes its context strictly inside A's) and asserts each observer sees exactly its own batch with nothing escaping. Verified to fail against emulated pre-fix shared-flag semantics.
• test_a_nested_bulk_update_resumes_the_enclosing_batch pins the nesting change.
• All threaded tests across both files: 20/20 runs clean, no flakiness.