Skip to content

fix: snapshot _connection_listeners before dispatch, not the live dict - #34

Merged
Bre77 merged 1 commit into
mainfrom
fm/pts-connlistener-mutation
Aug 12, 2026
Merged

fix: snapshot _connection_listeners before dispatch, not the live dict#34
Bre77 merged 1 commit into
mainfrom
fm/pts-connlistener-mutation

Conversation

@Bre77

@Bre77 Bre77 commented Aug 12, 2026

Copy link
Copy Markdown
Member

Intent

  • _update_connection_listeners() iterated self._connection_listeners.values() directly. A connection listener that calls get_vehicle() for an uncached VIN - e.g. an integration discovering a vehicle on reconnect - registers that vehicle's own connection listener from TeslemetryStreamVehicle.__init__, mutating _connection_listeners mid-iteration and raising RuntimeError: dictionary changed size during iteration, which interrupts connect()/disconnect notification delivery for every other listener still due to run.
    • Reproduced directly against _update_connection_listeners() before fixing, and via tests/test_stream_lifecycle.py's new test through the real connect() path.
  • Fix: iterate a list(...) snapshot instead of the live dict view - the same fix listen() already applies to _listeners for the identical hazard (see AGENTS.md), just without the sort/ordering requirement, since connection listeners don't share a mutable event object to race over.
  • tests/test_stream_lifecycle.py adds a case mirroring the existing _listeners mid-dispatch coverage: a connection listener discovering a vehicle mid-dispatch must not raise, and both the discovery and the new vehicle's own listener registration must still land correctly.

A connection listener that calls get_vehicle() for an uncached VIN (e.g.
an integration discovering a vehicle on reconnect) registers that
vehicle's own connection listener from inside TeslemetryStreamVehicle
__init__, mutating _connection_listeners while
_update_connection_listeners() is iterating it. That raised
RuntimeError: dictionary changed size during iteration, interrupting
connect()/disconnect notification delivery.

listen() already dispatches over a snapshot of _listeners for the same
reason; _update_connection_listeners() now does the same for
_connection_listeners.

Reported by Codex review on #33.
@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

if req.status == 404:
# No config exists for this vehicle yet - an authoritative
# answer (empty), not a missing one.
self._populated = True

P2 Badge Clear stale config when treating a 404 as authoritative

When a vehicle retained fields or preferTyped from before a disconnect and the reconnect-window REST fetch now returns 404 because that config was removed, this branch marks the record populated without clearing those stale values. add_field() or prefer_typed() can consequently treat the old value as current and return without sending the PATCH, leaving the requested telemetry configuration disabled indefinitely; reset both cached values to their no-config defaults before setting _populated.

AGENTS.md reference: AGENTS.md:L21-L21

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Bre77
Bre77 merged commit 7b4c9cf into main Aug 12, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant