fix(dcl): discard in-flight subscribe results for unsubscribed tags; release the orphaned handle

2026-08-14 23:25:48 -04:00
parent b1de9dfdd4
commit 37f13e2eaa
3 changed files with 208 additions and 7 deletions
@@ -288,4 +288,139 @@ public class DataConnectionActorBatchTests : TestKit
            () => adapter.UnsubscribeBatches.Any(b => b.Count == 8),
            TimeSpan.FromSeconds(10));
    }
+
+    [Fact]
+    public void ResolutionProbe_CompletingAfterUnsubscribe_IsDiscarded_AndItsHandleIsReleased()
+    {
+        // A tag unsubscribed while its resolution probe is in flight must NOT have the
+        // probe's result applied: HandleUnsubscribe already dropped it from
+        // _unresolvedTags and from TotalSubscribedTags, so storing the handle would push
+        // ResolvedTags above TotalSubscribedTags forever (and drive TotalSubscribedTags
+        // negative on the next redeploy) while leaking the adapter monitored item, which
+        // no later unsubscribe could ever reference.
+        var options = Options();
+        options.SubscribeBatchSize = 10;
+        // Long enough that the probe cannot fire before the gate below is armed.
+        options.TagResolutionRetryInterval = TimeSpan.FromMilliseconds(800);
+        options.TagResolutionRetryMaxInterval = TimeSpan.FromSeconds(2);
+
+        var adapter = new FakeBatchDataConnection();
+        adapter.FailingTags.Add("tag1");
+        var actor = CreateActor(adapter, options, "batch-probe-unsubscribe-race");
+
+        actor.Tell(new SubscribeTagsRequest(
+            "c1", "inst1", "batch-probe-unsubscribe-race", ["tag1"], DateTimeOffset.UtcNow));
+        ExpectMsg<TagValueUpdate>(u => u.Quality == QualityCode.Bad, TimeSpan.FromSeconds(5));
+        ExpectMsg<SubscribeTagsResponse>(TimeSpan.FromSeconds(5));
+
+        var afterSubscribe = Health(actor);
+        Assert.Equal(1, afterSubscribe.TotalSubscribedTags);
+        Assert.Equal(0, afterSubscribe.ResolvedTags);
+
+        // Park the next probe inside the adapter, and let it succeed when released.
+        var gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
+        adapter.SubscribeGate = gate.Task;
+        adapter.FailingTags.Clear();
+
+        var beforeProbe = adapter.SubscribeBatches.Count;
+        AwaitCondition(() => adapter.SubscribeBatches.Count > beforeProbe, TimeSpan.FromSeconds(10));
+
+        // Unsubscribe while the probe is still in flight, and confirm it has been applied
+        // before the probe result lands (same-sender ordering makes the report a barrier).
+        actor.Tell(new UnsubscribeTagsRequest("c2", "inst1", "batch-probe-unsubscribe-race", DateTimeOffset.UtcNow));
+        var afterUnsubscribe = Health(actor);
+        Assert.Equal(0, afterUnsubscribe.TotalSubscribedTags);
+        Assert.Equal(0, afterUnsubscribe.ResolvedTags);
+        // The tag was still unresolved, so the unsubscribe itself had no handle to
+        // release — every id released from here on is the raced probe's. (This also
+        // rules out a vacuous run in which the probe resolved before the gate was armed.)
+        Assert.Empty(adapter.UnsubscribeBatches);
+
+        gate.SetResult();
+
+        // The orphaned handle is released back to the adapter…
+        AwaitCondition(
+            () => adapter.UnsubscribeBatches.Any(b => b.Count == 1),
+            TimeSpan.FromSeconds(10));
+
+        // …and the counters are untouched by the discarded result.
+        AwaitAssert(() =>
+        {
+            var report = Health(actor);
+            Assert.Equal(0, report.TotalSubscribedTags);
+            Assert.Equal(0, report.ResolvedTags);
+        }, TimeSpan.FromSeconds(5));
+
+        // A redeploy round trip still books exactly one tag in and one tag out — pre-fix
+        // the stale handle made this read Total=0/Resolved=1 and then Total=-1.
+        adapter.SubscribeGate = null;
+        actor.Tell(new SubscribeTagsRequest(
+            "c3", "inst1", "batch-probe-unsubscribe-race", ["tag1"], DateTimeOffset.UtcNow));
+        ExpectMsg<SubscribeTagsResponse>(m => m.Success, TimeSpan.FromSeconds(5));
+
+        var afterRedeploy = Health(actor);
+        Assert.Equal(1, afterRedeploy.TotalSubscribedTags);
+        Assert.Equal(1, afterRedeploy.ResolvedTags);
+
+        actor.Tell(new UnsubscribeTagsRequest("c4", "inst1", "batch-probe-unsubscribe-race", DateTimeOffset.UtcNow));
+        var afterFinalUnsubscribe = Health(actor);
+        Assert.Equal(0, afterFinalUnsubscribe.TotalSubscribedTags);
+        Assert.Equal(0, afterFinalUnsubscribe.ResolvedTags);
+    }
+
+    [Fact]
+    public void ReconnectResubscribe_CompletingAfterUnsubscribe_IsDiscarded_AndItsHandleIsReleased()
+    {
+        // Same race on the OTHER batch source: the reconnect re-subscribe. ReSubscribeAll
+        // has already zeroed ResolvedTags, so applying the late result would report a
+        // resolved tag for an instance that no longer exists and leak its handle.
+        var options = Options();
+        options.SubscribeBatchSize = 10;
+
+        var adapter = new FakeBatchDataConnection();
+        var actor = CreateActor(adapter, options, "batch-resubscribe-unsubscribe-race");
+
+        actor.Tell(new SubscribeTagsRequest(
+            "c1", "inst1", "batch-resubscribe-unsubscribe-race", ["tag1"], DateTimeOffset.UtcNow));
+        ExpectMsg<SubscribeTagsResponse>(m => m.Success, TimeSpan.FromSeconds(5));
+
+        var gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
+        adapter.SubscribeGate = gate.Task;
+
+        var beforeReconnect = adapter.SubscribeBatches.Count;
+        adapter.RaiseDisconnected();
+        AwaitCondition(() => adapter.SubscribeBatches.Count > beforeReconnect, TimeSpan.FromSeconds(10));
+
+        actor.Tell(new UnsubscribeTagsRequest(
+            "c2", "inst1", "batch-resubscribe-unsubscribe-race", DateTimeOffset.UtcNow));
+        // The unsubscribe found no handle to release (ReSubscribeAll cleared
+        // _subscriptionIds), so no release round trip has happened yet.
+        Health(actor);
+        Assert.Empty(adapter.UnsubscribeBatches);
+
+        gate.SetResult();
+
+        // The handle minted by the re-subscribe is released rather than leaked, and the
+        // discarded row books no resolved tag. (TotalSubscribedTags across a reconnect
+        // window is a separate, pre-existing accounting gap — ReSubscribeAll clears the
+        // maps HandleUnsubscribe decrements from — so this pins the invariant that
+        // matters here: ResolvedTags never exceeds it.)
+        AwaitCondition(
+            () => adapter.UnsubscribeBatches.Any(b => b.Count == 1),
+            TimeSpan.FromSeconds(10));
+
+        AwaitAssert(() =>
+        {
+            var report = Health(actor);
+            Assert.Equal(0, report.ResolvedTags);
+            Assert.True(report.ResolvedTags <= report.TotalSubscribedTags);
+        }, TimeSpan.FromSeconds(5));
+    }
+
+    /// <summary>Point-in-time counters straight off the actor, fished past any pending pushes.</summary>
+    private DataConnectionHealthReport Health(IActorRef actor)
+    {
+        actor.Tell(new DataConnectionActor.GetHealthReport());
+        return FishForMessage<DataConnectionHealthReport>(_ => true, TimeSpan.FromSeconds(5));
+    }
 }