DriverInstanceActor: in-place ApplyDelta while Connected leaves stale subscription handles #663

New Issue

2026-10-09T07:46:00-04:00

dohertj2 commented

2026-10-09 07:46:00 -04:00

Follow-up from #662 (review finding 3, deliberately left unfixed there).

Defect (latent). DriverInstanceActor.HandleApplyDeltaAsync (Connected state) calls IDriver.ReinitializeAsync and changes nothing else. Every driver's reinit releases its device/session state (#630), so the driver-side data and native-alarm subscriptions are gone — but the actor keeps _subscriptionHandle / _alarmSubscriptionHandle. ReconcileSubscription / ReconcileAlarmSubscription see non-null handles and never re-subscribe, so every tag and alarm on that driver goes silent until a reconnect or respawn.

Reachability. Not reachable from deploys today: since #516 a DriverConfig change is stop + respawn, so DriverHostActor.ApplyChildDelta is never called on the reconcile path. The Connecting/Reconnecting ApplyDelta (AdoptConfigDuringInit) is fine — handles are already detached there.

Proposed fix. Route a Connected ApplyDelta through the existing reconnect path (same as ForceReconnect): DetachSubscription(), adopt the new config, Become(Reconnecting), init with the new config; the Connected entry then re-subscribes data + alarms fresh. Ack semantics match AdoptConfigDuringInit (acked on adoption).

Tests. ApplyDelta to a Connected driver ⇒ fresh data + alarm subscriptions and an alarm still reaches its condition; falsified by restoring the old handler.

Follow-up from #662 (review finding 3, deliberately left unfixed there). **Defect (latent).** `DriverInstanceActor.HandleApplyDeltaAsync` (Connected state) calls `IDriver.ReinitializeAsync` and changes nothing else. Every driver's reinit releases its device/session state (#630), so the driver-side data and native-alarm subscriptions are gone — but the actor keeps `_subscriptionHandle` / `_alarmSubscriptionHandle`. `ReconcileSubscription` / `ReconcileAlarmSubscription` see non-null handles and never re-subscribe, so every tag and alarm on that driver goes silent until a reconnect or respawn. **Reachability.** Not reachable from deploys today: since #516 a DriverConfig change is stop + respawn, so `DriverHostActor.ApplyChildDelta` is never called on the reconcile path. The Connecting/Reconnecting `ApplyDelta` (`AdoptConfigDuringInit`) is fine — handles are already detached there. **Proposed fix.** Route a Connected `ApplyDelta` through the existing reconnect path (same as `ForceReconnect`): `DetachSubscription()`, adopt the new config, `Become(Reconnecting)`, init with the new config; the Connected entry then re-subscribes data + alarms fresh. Ack semantics match `AdoptConfigDuringInit` (acked on adoption). **Tests.** ApplyDelta to a Connected driver ⇒ fresh data + alarm subscriptions and an alarm still reaches its condition; falsified by restoring the old handler.

dohertj2 referenced this issue from a commit

2026-10-09 08:00:12 -04:00

fix(runtime): in-place ApplyDelta releases and re-subscribes data + alarms (#663)

dohertj2 referenced this issue from a commit

2026-10-09 08:00:12 -04:00

Merge hw6-663 (#663 in-place ApplyDelta re-subscribes data + alarms)

dohertj2 commented

2026-10-09 08:00:21 -04:00

Fixed on master in 1355049f (merging 39c5af3d).

Fix. A Connected ApplyDelta now:

Releases the data subscription and the native-alarm subscription at the driver (bounded and best-effort), and drops the actor's handlers and handles.
Calls ReinitializeAsync.
Re-subscribes both in a finally, because a failed reinit has lost them too.

Why reinit and not the reconnect path the issue proposed: CalculationDriver and GalaxyDriver take their config at construction and ignore the JSON passed to InitializeAsync. Shutdown + initialise would therefore silently not adopt the new config for them. The ack and DeltaApplied ordering are unchanged.

Tests. An_in_place_ApplyDelta_leaves_one_live_data_and_one_live_alarm_subscription has two cases, and each fails against a different wrong fix:

The reinit drops subscriptions (the production shape). This case fails on the old handler.
The reinit keeps subscriptions. This case fails on a fix that only nulls the handles without releasing them, because the driver ends up holding two.

The "not two" check waits for three counted reconcile ticks before asserting. Full suite on the merged master: 13216 passed, 0 failed.

Still latent: no deploy reaches this path since #516. Docs: docs/DriverLifecycle.md, CLAUDE.md. The scadaproj history entry is 484b176; ScadaBridge impact is none.

Fixed on master in `1355049f` (merging `39c5af3d`). **Fix.** A Connected `ApplyDelta` now: 1. Releases the data subscription and the native-alarm subscription at the driver (bounded and best-effort), and drops the actor's handlers and handles. 2. Calls `ReinitializeAsync`. 3. Re-subscribes both in a `finally`, because a failed reinit has lost them too. **Why reinit and not the reconnect path the issue proposed:** `CalculationDriver` and `GalaxyDriver` take their config at construction and ignore the JSON passed to `InitializeAsync`. Shutdown + initialise would therefore silently not adopt the new config for them. The ack and `DeltaApplied` ordering are unchanged. **Tests.** `An_in_place_ApplyDelta_leaves_one_live_data_and_one_live_alarm_subscription` has two cases, and each fails against a different wrong fix: - **The reinit drops subscriptions (the production shape).** This case fails on the old handler. - **The reinit keeps subscriptions.** This case fails on a fix that only nulls the handles without releasing them, because the driver ends up holding two. The "not two" check waits for three counted reconcile ticks before asserting. Full suite on the merged master: 13216 passed, 0 failed. Still latent: no deploy reaches this path since #516. Docs: `docs/DriverLifecycle.md`, `CLAUDE.md`. The scadaproj history entry is `484b176`; ScadaBridge impact is none.

dohertj2 closed this issue

2026-10-09 08:00:21 -04:00

dohertj2 referenced this issue

2026-10-09 11:01:24 -04:00

Mid-connect ApplyDelta: Calculation and Galaxy ignore the adopted config; no DeltaApplied re-register #664

dohertj2 referenced this issue

2026-10-09 11:20:02 -04:00

Delete the dead in-place ApplyDelta seam (config change = respawn only) #665

dohertj2 referenced this issue from a commit

2026-10-09 11:32:54 -04:00

refactor(runtime): delete the dead in-place ApplyDelta seam; config change = respawn only (#665)

dohertj2 referenced this issue