Skip to content

fix: reset MessageListener.stopped when re-adding to a notifier - #123

Closed
drewr95 wants to merge 147 commits into
juergenH87:masterfrom
RaulSMS:fix/stale-listener-stopped-flag
Closed

fix: reset MessageListener.stopped when re-adding to a notifier#123
drewr95 wants to merge 147 commits into
juergenH87:masterfrom
RaulSMS:fix/stale-listener-stopped-flag

Conversation

@drewr95

@drewr95 drewr95 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Summary

can.Notifier.stop() calls listener.stop() on every listener it holds,
which sets MessageListener.stopped = True permanently -- nothing ever
resets it. ElectronicControlUnit creates its listener once in __init__
and reuses it for the ECU's whole lifetime.

The ECU and its Notifier are governed by separate ref-counted registries in
consuming code (e.g. j1939_utilities' EcuRegistry/NotifierRegistry). If
a shared notifier's refcount independently hits zero while the ECU itself
survives (some other consumer still holds an ECU reference), the notifier
gets torn down and a fresh one created -- but add_notifier() was re-adding
the ECU's same, already-stopped=True listener to the new notifier without
clearing the flag, silently and permanently dropping every future frame for
that ECU even though the notifier is alive and the listener is registered
on it.

Fix

Reset listener.stopped = False in add_notifier() before re-adding it to
the (possibly new) notifier.

Context

Found while investigating boom_integration_tests rotary/tilt startup test
flakiness at full-suite scale (downstream consumer, pinned to a commit on
this fork). Turned out not to be the actual root cause there (a stray CAN
bus filter left by an unrelated fixture in a different repo), but this is a
real, independently reproducible bug in its own right -- confirmed present
at the currently-pinned commit.

Testing

No local test env for this repo in my sandbox; happy to add a regression
test if wanted, or if pointed at the right test scaffolding.

RaulSMS and others added 30 commits May 20, 2026 12:54
To support agentic development
Extend the DTC class and Dm1 sender/receiver to handle SAE J1939-73 SPN
conversion methods 1, 2, 3, and 4 (previously only CM 4 / CM-bit-clear
was supported and other methods were logged as errors on receive). TX
takes an optional per-DTC 'cm' key (default 4); RX disambiguates the
CM-bit-set case via a new Dm1(rx_cm_bit_set=...) constructor arg.
Add support for all four DM1 SPN conversion methods
* feat: remove numpy

* test: add coverage for j1939_22 logic

* test: clean up docs and remove constant

* feat: use constants instead of hardcoded numbers
With the two-thread model introduced in this branch, the protocol thread
iterates _snd_buffer in async_job_thread concurrently with send_pgn being
called from a user/timer thread. The check-then-write on _snd_buffer was
unprotected, creating a live race (RuntimeError: dictionary changed size
during iteration on CPython).

j1939_22.py already wraps its send_pgn buffer writes with _buffer_lock;
this commit brings j1939_21.py to the same standard.

CAN I/O (_send_tp_bam / _send_tp_rts) is intentionally kept outside the
lock to avoid holding it during I/O.
Two new tests in test_threading.py covering the race condition fixed in
the previous commit:

- test_send_pgn_concurrent_no_crash: 4 threads hammer send_pgn while the
  protocol thread is running; verifies no RuntimeError or crash.
- test_send_pgn_j1939_21_buffer_lock_no_race: two threads race to send to
  the same src/dst pair simultaneously; verifies the check-then-write is
  atomic (exactly one succeeds, one is rejected).
#11)

python-can v4.2.0 renamed the Bus() kwarg from 'bustype' to 'interface'
and scheduled 'bustype' for removal in v5.0. Update all examples and the
connect() docstring, and bump the minimum version requirement in setup.py.

Fixes #9.
- Remove test_helpers/__init__.py so find_packages() no longer picks it
  up; feeder.py stays in test_helpers/ as the contributor prefers.
- Move conftest.py to the repo root so pytest auto-discovers the feeder
  fixture — no more explicit `from test_helpers.conftest import feeder`
  in every test file.
- Add test_helpers to find_packages(exclude=...) as an explicit guard.
- Drop the now-redundant explicit fixture imports from all test files.

Fixes the issue documented in reviews/issue-test-folder-structure.md.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…12)

Covers issue #8 — device_address_preferred=0 is falsy in Python, so a
bare truthiness check silently skipped the bypass, leaving the CA in
State.NONE. The fix (is not None guard) was already applied; this test
pins the behaviour so it cannot regress.
…ments

General threading and other misc. improvements
khauersp and others added 28 commits June 30, 2026 14:02
…1 and J1939_22 classes; add tests for passive observation behavior #59
# Conflicts:
#	j1939/j1939_21.py
#	j1939/j1939_22.py
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4.6.1 to 7.0.1.
- [Release notes](https://github.com/actions/upload-artifact/releases)
- [Commits](actions/upload-artifact@4cec3d8...043fb46)

---
updated-dependencies:
- dependency-name: actions/upload-artifact
  dependency-version: 7.0.1
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
…actions/upload-artifact-7.0.1

chore(deps): bump actions/upload-artifact from 4.6.1 to 7.0.1
…ows/generator_generic_slsa3.yml

Bumps [slsa-framework/slsa-github-generator/.github/workflows/generator_generic_slsa3.yml](https://github.com/slsa-framework/slsa-github-generator) from 2.0.0 to 2.1.0.
- [Release notes](https://github.com/slsa-framework/slsa-github-generator/releases)
- [Changelog](https://github.com/slsa-framework/slsa-github-generator/blob/main/CHANGELOG.md)
- [Commits](slsa-framework/slsa-github-generator@v2.0.0...v2.1.0)

---
updated-dependencies:
- dependency-name: slsa-framework/slsa-github-generator/.github/workflows/generator_generic_slsa3.yml
  dependency-version: 2.1.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
…slsa-framework/slsa-github-generator/dot-github/workflows/generator_generic_slsa3.yml-2.1.0

chore(deps): bump slsa-framework/slsa-github-generator/.github/workflows/generator_generic_slsa3.yml from 2.0.0 to 2.1.0
…s-on-construction

Feature/add support for bus on construction
Use Dispatch thread for processing
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 6.3.0 to 7.0.0.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](actions/setup-python@v6.3.0...v7.0.0)

---
updated-dependencies:
- dependency-name: actions/setup-python
  dependency-version: 7.0.0
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
…actions/setup-python-7.0.0

chore(deps): bump actions/setup-python from 6.3.0 to 7.0.0
Bumps [actions/checkout](https://github.com/actions/checkout) from 7.0.0 to 7.0.1.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@9c091bb...3d3c42e)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: 7.0.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
…actions/checkout-7.0.1

chore(deps): bump actions/checkout from 7.0.0 to 7.0.1
can.Notifier.stop() calls listener.stop() on every listener it holds,
which sets MessageListener.stopped = True permanently -- nothing ever
resets it. ElectronicControlUnit creates its listener once in __init__
and reuses it for the ECU's whole lifetime.

The ECU and its Notifier are governed by separate ref-counted registries
in consuming code (e.g. j1939_utilities' EcuRegistry/NotifierRegistry).
If a shared notifier's refcount independently hits zero while the ECU
itself survives (some other consumer still holds an ECU reference), the
notifier gets torn down and a fresh one created, but add_notifier() was
re-adding the ECU's same, already-stopped=True listener to the new
notifier without clearing the flag -- silently and permanently dropping
every future frame for that ECU, even though the notifier is alive and
the listener is registered on it.

Found while investigating boom_integration_tests rotary/tilt startup
test flakiness at full-suite scale. Turned out not to be the actual
root cause there (a stray CAN bus filter left by an unrelated fixture),
but this is a real, independently reproducible bug in its own right.
@drewr95

drewr95 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Wrong target repo — recreating against RaulSMS/python-can-j1939, which is the fork this project actually depends on.

@drewr95 drewr95 closed this Aug 14, 2026
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.

6 participants