fix(ablegacy): prune per-key gates when an indirect index moves

Review 07 L7: each index value an indirect tag (N7:[N7:0]) swept through left a
per-runtime and a creation SemaphoreSlim in the device's gate tables until
shutdown. A moved index now drops the stale resolution's gates along with its
handle, and sweeps any other stale resolution of the same tag a concurrent
reader cached, so the tables stay bounded by the authored set.

Refs #738
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
2026-10-22 14:17:16 -04:00
parent 92171a01ec
commit 7377daa9b0
2 changed files with 56 additions and 2 deletions
@@ -965,8 +965,8 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
var concreteText = concrete.ToLibplctagName();
var resolvedName = $"{def.Name}=>{concreteText}";
// The index moved: drop the previous resolution's handle so a sweeping index does not accumulate one
// libplctag handle per value it ever took.
// The index moved: drop the previous resolution's handle AND its per-key gates (review 07 L7), so a sweeping
// index does not accumulate one libplctag handle plus two SemaphoreSlims per value it ever took.
var previous = device.IndirectLastName.TryGetValue(def.Name, out var prior) ? prior : null;
device.IndirectLastName[def.Name] = resolvedName;
if (previous is not null && !string.Equals(previous, resolvedName, StringComparison.OrdinalIgnoreCase))
@@ -974,6 +974,17 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
var oldLock = device.GetRuntimeLock(previous);
await oldLock.WaitAsync(ct).ConfigureAwait(false);
try { EvictRuntime(device, previous); } finally { oldLock.Release(); }
device.ForgetGates(previous);
// A concurrent reader of the same indirect tag can have cached a resolution between our TryGetValue and
// the write above (a stale one nothing would ever evict). Sweep every OTHER resolution of this tag.
var prefix = def.Name + "=>";
foreach (var key in device.Runtimes.Keys)
{
if (!key.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)
|| string.Equals(key, resolvedName, StringComparison.OrdinalIgnoreCase)) continue;
EvictRuntime(device, key);
device.ForgetGates(key);
}
}
return new IndirectResolution(def with { Name = resolvedName, Address = concreteText }, 0, false);
}
@@ -1379,6 +1390,21 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
/// <summary>Tags already warned about for a malformed address (once-per-tag log).</summary>
public System.Collections.Concurrent.ConcurrentDictionary<string, byte> MalformedAddressLogged { get; } = new();
/// <summary>
/// Drops the per-runtime and creation gates of a runtime key that is no longer in use (a moved indirect
/// resolution). The gates are removed, not disposed: a caller that already holds a reference finishes on
/// it, and the next user of the key simply gets a fresh gate.
/// </summary>
/// <param name="runtimeKey">The runtime key whose gates to drop.</param>
public void ForgetGates(string runtimeKey)
{
_runtimeLocks.TryRemove(runtimeKey, out _);
_creationLocks.TryRemove($"tag:{runtimeKey}", out _);
}
/// <summary>The number of per-key gates currently held (runtime + creation + RMW). Tests only.</summary>
internal int GateCount => _runtimeLocks.Count + _creationLocks.Count + _rmwLocks.Count;
private volatile bool _retired;
/// <summary>True once this state has been unpublished and released; nothing may be cached on it any more.</summary>
@@ -128,4 +128,32 @@ public sealed class AbLegacyIndirectAddressingTests
w.StatusCode.ShouldBe(AbLegacyStatusMapper.BadOutOfRange);
f.Tags.Values.ShouldAllBe(t => t.WriteCount == 0);
}
/// <summary>
/// Review 07 L7: every index value an indirect tag sweeps through used to leave one per-runtime semaphore and
/// one creation semaphore behind until shutdown. They are pruned with the stale resolution's handle, so the
/// per-key gate tables stay bounded by the authored set however far the index sweeps.
/// </summary>
[Fact]
public async Task Sweeping_index_does_not_accumulate_per_key_gates()
{
var (drv, f) = await NewDriverAsync(new AbLegacyTagDefinition("Ind", Host, "N7:[N7:0]", AbLegacyDataType.Int));
Seed(f, ("N7:0", (short)10));
(await drv.ReadAsync(["Ind"], CancellationToken.None)).Single().StatusCode.ShouldBe(AbLegacyStatusMapper.Good);
var state = drv.GetDeviceState(Host)!;
var baseline = state.GateCount;
for (short i = 11; i < 60; i++)
{
f.Tags["N7:0"].Value = i;
f.Customise = p => new FakeAbLegacyTag(p) { Value = (short)(p.TagName == "N7:0" ? i : 1000 + i) };
var r = (await drv.ReadAsync(["Ind"], CancellationToken.None)).Single();
r.StatusCode.ShouldBe(AbLegacyStatusMapper.Good); // positive control: every resolution really read
r.Value.ShouldBe((short)(1000 + i));
}
state.GateCount.ShouldBe(baseline);
// Only the live resolution's handle (plus the index word's parent runtime) remains cached.
state.Runtimes.Keys.Count(k => k.StartsWith("Ind=>", StringComparison.Ordinal)).ShouldBe(1);
}
}