GroupedPropertyDict guards its dictionaries with an RLock, but BulkUpdateContext.__enter__ and __exit__ mutate _bulk_mode and _bulk_context on the shared dict with no lock held. Those two attributes decide whether property changes accumulate into one batched event or fire individually, so two threads entering bulk contexts on the same GroupedPropertyDict interleave and corrupt each other's batches.
Every other accessor on the class takes the lock — including the observer dispatch and the group scans — which makes the omission look accidental rather than deliberate.
What goes wrong
__enter__ sets _bulk_mode = True and _bulk_context = self; __exit__ sets them back to False/None. With two threads, A entering while B is inside its context replaces _bulk_context, and whichever exits first clears _bulk_mode for both.
The consequences are not symmetric, and the first one is easy to over-state, so to be precise:
- Events are not lost.
__exit__ fires self.pending_events, the context object's own list, so a displaced context still flushes its own batch when it exits.
- Events are misattributed and fragmented. Some of thread A's changes land in thread B's batch, and changes made after B exits fire individually as separate events instead of being batched.
For a consumer that treats a batch as an atomic structural update, that fragmentation is the problem. In a Homie publisher, structural events escaping the batch each trigger their own device-description republish and a device state transition, so a single logical change produces extra $description republishes and visible $state flapping.
Reproducing conditions
Two threads entering bulk contexts on the same GroupedPropertyDict is not exotic. It arises whenever a periodic maintenance task and an inbound-message handler both update the model, which is an ordinary shape for a publisher: one thread applies incoming state at whatever cadence the source produces it, another runs a slower housekeeping pass.
Any blocking call inside the critical section widens the window considerably, since it releases the GIL and lets the other thread run precisely while the flags are inconsistent.
Suggested fix
Either make _bulk_mode and _bulk_context thread-local, so concurrent bulk contexts on the same dict are independent, or hold the existing RLock across __enter__ and __exit__ so they serialize.
Thread-local is the better fit: two threads batching their own updates independently is reasonable behaviour, whereas serializing means one blocks for the duration of the other's context, which could be long if it contains I/O.
Worth adding a test that enters two bulk contexts from different threads and asserts each observer sees exactly its own batch.
GroupedPropertyDictguards its dictionaries with anRLock, butBulkUpdateContext.__enter__and__exit__mutate_bulk_modeand_bulk_contexton the shared dict with no lock held. Those two attributes decide whether property changes accumulate into one batched event or fire individually, so two threads entering bulk contexts on the sameGroupedPropertyDictinterleave and corrupt each other's batches.Every other accessor on the class takes the lock — including the observer dispatch and the group scans — which makes the omission look accidental rather than deliberate.
What goes wrong
__enter__sets_bulk_mode = Trueand_bulk_context = self;__exit__sets them back toFalse/None. With two threads, A entering while B is inside its context replaces_bulk_context, and whichever exits first clears_bulk_modefor both.The consequences are not symmetric, and the first one is easy to over-state, so to be precise:
__exit__firesself.pending_events, the context object's own list, so a displaced context still flushes its own batch when it exits.For a consumer that treats a batch as an atomic structural update, that fragmentation is the problem. In a Homie publisher, structural events escaping the batch each trigger their own device-description republish and a device state transition, so a single logical change produces extra
$descriptionrepublishes and visible$stateflapping.Reproducing conditions
Two threads entering bulk contexts on the same
GroupedPropertyDictis not exotic. It arises whenever a periodic maintenance task and an inbound-message handler both update the model, which is an ordinary shape for a publisher: one thread applies incoming state at whatever cadence the source produces it, another runs a slower housekeeping pass.Any blocking call inside the critical section widens the window considerably, since it releases the GIL and lets the other thread run precisely while the flags are inconsistent.
Suggested fix
Either make
_bulk_modeand_bulk_contextthread-local, so concurrent bulk contexts on the same dict are independent, or hold the existingRLockacross__enter__and__exit__so they serialize.Thread-local is the better fit: two threads batching their own updates independently is reasonable behaviour, whereas serializing means one blocks for the duration of the other's context, which could be long if it contains I/O.
Worth adding a test that enters two bulk contexts from different threads and asserts each observer sees exactly its own batch.