Skip to content

Stop monitors racing their own subscription when the channel closes - #47

Merged
slominskir merged 1 commit into
mainfrom
fix-monitor-subscription-race
Oct 4, 2026
Merged

slominskir merged 1 commit into
mainfrom
fix-monitor-subscription-race

Conversation

@slominskir-coding-agent

Copy link
Copy Markdown
Contributor

Fixes the intermittent CaGetConcurrencyTest failure in CI on main (run 53: "Monitors that never reported connected expected:<0> but was:<75>").

A cloud Claude session diagnosed the failure and wrote this commit (its session is linked in the commit trailer). I reviewed it, checked its claims, and applied it unchanged except for setting the author to the repository's bot.

Cause

Since #45, a /caget and a monitor of the same PV share one CAJ channel.

  • What drops the circuit: CAJ queues a monitor's EVENT_ADD until a flush, but sends EVENT_CANCEL at once (EventCancelRequest.getPriority() is SEND_IMMEDIATELY_PRIORITY; I checked the bytecode of jca 2.4.10). A cancel that closely follows its add can therefore reach the IOC first. The IOC rejects it and drops the whole circuit, with every channel on that IOC.
  • Why ChannelMonitor made that likely: it never removed its connection listener or cleared its subscription on close(). On a channel a /caget kept open, a closed monitor could still subscribe when its connection callback ran late, and subscriptions piled up. When the last /caget then destroyed the channel, CAJ cancelled every subscription still on it, including that fresh one.
  • Why one drop became 75 failures: when the drop landed while a monitor was closing, destroyChannel threw IllegalStateException. close() didn't catch it, so it escaped removePv's computeIfPresent and left the dead monitor mapped. Every later client of the PV was then told it was disconnected.

The CI run's server log matches: the IOC logged CAS: forcing disconnect, then epics2web logged transport closed and Unable to destroy channel channel2, followed by 76 Unable to handle client message lines.

Changes

ChannelMonitor.close() now:

  • stops the monitor subscribing after close;
  • removes its connection listener from the possibly shared channel;
  • clears its own subscription once the first update shows the IOC has it, waiting at most 3 s;
  • reports IllegalStateException as IOException, so removePv always unmaps the monitor.

MonitorEndpoint now logs the message of an IllegalStateException, which its log call used to drop.

Waiting inside computeIfPresent: the first update arrives on a CAJ thread, and nothing on that path takes monitorMap locks. Connection events go through callbackExecutor, and the closing monitor has no listeners left. The wait happens only while the channel is connected with an unconfirmed subscription, which is normally milliseconds.

Checks

Run locally with Tomcat CPU-limited (docker update --cpus) to make the race likelier:

Build CPU Runs of CaGetConcurrencyTest Failures IOC forcing disconnect
main (61761e6) 1 10 0 0
main 0.5 15 0 1
main 0.3 15 0 1
this PR 1 10 0 0
this PR 0.3 30 0 0
  • What this does and doesn't show: the trigger (a forced disconnect) appears about once in 15 runs on main, and never in 30 with this PR. The 75-monitor failure itself didn't reproduce locally, so this PR's CI runs are the real confirmation.
  • Other checks:
    • The full integration suite passed twice at normal CPU, with timings unchanged (CaGetConcurrencyTest 3.0 s).
    • ./gradlew spotlessCheck test passes.
  • Still logged: User destroyed channel (7 in the patched runs). The handoff found these come from CAJ destroying a shared channel while a get is pending, before and after this change. They don't drop the circuit.

🤖 Generated with Claude Code

CaGetConcurrencyTest failed in CI on main (75 monitors never reported
connected). A packet capture of the test against the IOC shows why.
CAJ queues a monitor's EVENT_ADD until a flush but sends EVENT_CANCEL
at once, so a cancel that closely follows the add reaches the IOC
first. The IOC rejects it (bad monitor subscription identifier) and
drops the connection, with every channel on it.

ChannelMonitor made that likely. It never removed its connection
listener or cleared its subscription, so on a channel a /caget kept
open, a closed monitor still subscribed when its connection callback
ran late. When the last /caget then destroyed the channel, CAJ
cancelled that fresh subscription. Subscriptions also piled up on the
shared channel, one per monitor.

When the drop landed while a monitor was closing, destroyChannel threw
IllegalStateException, which close() didn't catch. It escaped
removePv's compute, leaving the dead monitor mapped, and every later
client of that PV was told it was disconnected.

ChannelMonitor.close() now:
- stops the monitor subscribing after close, and removes its
  connection listener from the shared channel;
- clears its own subscription, after the first update shows the IOC
  has it (waiting at most 3 s);
- reports IllegalStateException as IOException too, so removePv always
  unmaps the monitor.

MonitorEndpoint also logs the message of an IllegalStateException,
which its log call dropped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VbHof7Yu45SnHgQB9V1CwD
@slominskir
slominskir merged commit ce1000b into main Oct 4, 2026
6 checks passed
@slominskir
slominskir deleted the fix-monitor-subscription-race branch October 4, 2026 03:13
slominskir pushed a commit that referenced this pull request Oct 4, 2026
AGENTS.md tells coding agents how to check their work here: the unit
and integration test commands, the test IOC's PVs and what tests rely
on, and the rules for code that uses CAJ that earlier bugs taught
(#29, #42, #47 and others). It also covers the conventions for
commits, pull requests and releases. Based on the coding-agents
starter file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

source::ai Work done by an AI agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant