diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs index 66db900d..b5bc01e2 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs @@ -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 /// Tags already warned about for a malformed address (once-per-tag log). public System.Collections.Concurrent.ConcurrentDictionary MalformedAddressLogged { get; } = new(); + /// + /// 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. + /// + /// The runtime key whose gates to drop. + public void ForgetGates(string runtimeKey) + { + _runtimeLocks.TryRemove(runtimeKey, out _); + _creationLocks.TryRemove($"tag:{runtimeKey}", out _); + } + + /// The number of per-key gates currently held (runtime + creation + RMW). Tests only. + internal int GateCount => _runtimeLocks.Count + _creationLocks.Count + _rmwLocks.Count; + private volatile bool _retired; /// True once this state has been unpublished and released; nothing may be cached on it any more. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyIndirectAddressingTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyIndirectAddressingTests.cs index 6baf2165..bd034822 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyIndirectAddressingTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyIndirectAddressingTests.cs @@ -128,4 +128,32 @@ public sealed class AbLegacyIndirectAddressingTests w.StatusCode.ShouldBe(AbLegacyStatusMapper.BadOutOfRange); f.Tags.Values.ShouldAllBe(t => t.WriteCount == 0); } + + /// + /// 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. + /// + [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); + } }