dohertj2/lmxopcua
1
0
Fork 0
Code Issues 32 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(ab): orphan release clears the wrapper handle; rooting fallback documented and pinned (T21 review r1 M1/M2)

Browse Source 
M2: after plc_tag_destroy, the release writes 0 back to the wrapper's nativeTagHandle, so a repeat release is a no-op instead of touching a destroyed id. The runtimes keep the destroyed handle for tests and diagnostics.

M1: the rooting fallback is now described accurately, as an unbounded leak (one native tag per failed create), not a bounded one. A test seam (ForceRootingForTest) pins that the fallback roots the tag and destroys nothing.

Both pins are red with their line removed. The orphan tests share a non-parallel collection, because the counters and the seam are process-wide.

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-07 20:16:33 -04:00
parent 43b81b6643
commit 6ae6287360
6 changed files with 100 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagOrphanRelease.cs
+13 -2
View File
@@ -23,7 +23,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip;
/// <para>The wrapper exposes no handle, so it is read by reflection from the two private fields of the pinned
/// 1.5.2 wrapper (<c>Tag._tag</c>, <c>NativeTagWrapper.nativeTagHandle</c>). If a future wrapper renames them the
/// release cannot run; the tag is then rooted for the life of the process instead, which keeps its delegate
/// alive (no crash, a bounded leak) — and <c>LibplctagOrphanReleaseTests</c> fails, so the bump is noticed.</para>
/// alive. That is no crash but an UNBOUNDED leak — one native tag (holding its libplctag session open) per failed
/// create, and the data path re-creates a NotFound tag on every read — so <c>LibplctagOrphanReleaseTests</c> fails, so the bump is noticed.</para>
/// </remarks>
internal static class LibplctagOrphanRelease
{
@@ -38,7 +39,10 @@ internal static class LibplctagOrphanRelease
private static int _released;
/// <summary>True when the pinned wrapper's private fields were found, so a failed tag can be destroyed.</summary>
internal static bool CanReadHandles => HandleField is not null;
internal static bool CanReadHandles => HandleField is not null && !ForceRootingForTest;
/// <summary>Test seam (review r1 M1): take the rooting fallback as if the wrapper's fields had been renamed.</summary>
internal static bool ForceRootingForTest { get; set; }
/// <summary>Orphaned native tags destroyed so far (process-wide; diagnostics and tests).</summary>
internal static int Released => Volatile.Read(ref _released);
@@ -50,6 +54,12 @@ internal static class LibplctagOrphanRelease
internal static int HandleOf(Tag tag) =>
HandleField is not null && WrapperField!.GetValue(tag) is { } wrapper ? (int)HandleField.GetValue(wrapper)! : 0;
/// <summary>Clears the wrapper's handle after a destroy, so a repeat release is a no-op (review r1 M2).</summary>
private static void ClearHandle(Tag tag)
{
if (HandleField is not null && WrapperField!.GetValue(tag) is { } wrapper) HandleField.SetValue(wrapper, 0);
}
/// <summary>
/// Runs a libplctag operation that may initialise <paramref name="tag"/> (InitializeAsync, or a first
/// Read/Write, which initialise on demand). If it fails and leaves the tag uninitialised, the native tag it
@@ -103,6 +113,7 @@ internal static class LibplctagOrphanRelease
var status = (Status)plctag.plc_tag_status(handle);
plctag.plc_tag_unregister_callback(handle);
plctag.plc_tag_destroy(handle);
ClearHandle(tag);
Interlocked.Increment(ref _released);
return status;
}
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs
+16 -3
View File
@@ -20,6 +20,9 @@ internal sealed class LibplctagTagRuntime : IAbCipTagRuntime
/// </summary>
private Status? _releasedStatus;
/// <summary>The native handle a failed initialisation destroyed (0 if none); the wrapper's own field is cleared.</summary>
private int _releasedHandle;
/// <summary>Initializes a new instance of the LibplctagTagRuntime class.</summary>
/// <param name="p">The tag creation parameters.</param>
public LibplctagTagRuntime(AbCipTagCreateParams p)
@@ -70,16 +73,26 @@ internal sealed class LibplctagTagRuntime : IAbCipTagRuntime
{
await LibplctagStatusTranslation.Wrap(op).ConfigureAwait(false);
_releasedStatus = null;
_releasedHandle = 0;
}
catch
{
if (LibplctagOrphanRelease.ReleaseIfOrphaned(_tag) is { } status) _releasedStatus = status;
var handle = LibplctagOrphanRelease.HandleOf(_tag);
if (LibplctagOrphanRelease.ReleaseIfOrphaned(_tag) is { } status)
{
_releasedStatus = status;
_releasedHandle = handle;
}
throw;
}
}
/// <summary>The native libplctag handle this runtime's tag holds or last held (0 if none was created). Tests only.</summary>
internal int NativeHandle => LibplctagOrphanRelease.HandleOf(_tag);
/// <summary>The native libplctag handle this runtime's tag holds, or the one a failed initialisation destroyed
/// (0 if none was created). Tests only.</summary>
internal int NativeHandle => _releasedHandle != 0 ? _releasedHandle : LibplctagOrphanRelease.HandleOf(_tag);
/// <summary>The handle the libplctag wrapper itself still holds (0 once a release cleared it). Tests only.</summary>
internal int WrapperHandle => LibplctagOrphanRelease.HandleOf(_tag);
/// <inheritdoc />
public int GetStatus() =>
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyTagRuntime.cs
+16 -3
View File
@@ -19,6 +19,9 @@ internal sealed class LibplctagLegacyTagRuntime : IAbLegacyTagRuntime
/// </summary>
private Status? _releasedStatus;
/// <summary>The native handle a failed initialisation destroyed (0 if none); the wrapper's own field is cleared.</summary>
private int _releasedHandle;
/// <summary>Initializes a new instance of the <see cref="LibplctagLegacyTagRuntime"/> class.</summary>
/// <param name="p">The parameters for tag creation.</param>
public LibplctagLegacyTagRuntime(AbLegacyTagCreateParams p)
@@ -66,16 +69,26 @@ internal sealed class LibplctagLegacyTagRuntime : IAbLegacyTagRuntime
{
await LibplctagLegacyStatusTranslation.Wrap(op).ConfigureAwait(false);
_releasedStatus = null;
_releasedHandle = 0;
}
catch
{
if (LibplctagOrphanRelease.ReleaseIfOrphaned(_tag) is { } status) _releasedStatus = status;
var handle = LibplctagOrphanRelease.HandleOf(_tag);
if (LibplctagOrphanRelease.ReleaseIfOrphaned(_tag) is { } status)
{
_releasedStatus = status;
_releasedHandle = handle;
}
throw;
}
}
/// <summary>The native libplctag handle this runtime's tag holds or last held (0 if none was created). Tests only.</summary>
internal int NativeHandle => LibplctagOrphanRelease.HandleOf(_tag);
/// <summary>The native libplctag handle this runtime's tag holds, or the one a failed initialisation destroyed
/// (0 if none was created). Tests only.</summary>
internal int NativeHandle => _releasedHandle != 0 ? _releasedHandle : LibplctagOrphanRelease.HandleOf(_tag);
/// <summary>The handle the libplctag wrapper itself still holds (0 once a release cleared it). Tests only.</summary>
internal int WrapperHandle => LibplctagOrphanRelease.HandleOf(_tag);
/// <inheritdoc />
public int GetStatus() =>
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagOrphanRelease.cs
+13 -2
View File
@@ -23,7 +23,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy;
/// <para>The wrapper exposes no handle, so it is read by reflection from the two private fields of the pinned
/// 1.5.2 wrapper (<c>Tag._tag</c>, <c>NativeTagWrapper.nativeTagHandle</c>). If a future wrapper renames them the
/// release cannot run; the tag is then rooted for the life of the process instead, which keeps its delegate
/// alive (no crash, a bounded leak) — and <c>LibplctagLegacyOrphanReleaseTests</c> fails, so the bump is noticed.</para>
/// alive. That is no crash but an UNBOUNDED leak — one native tag (holding its libplctag session open) per failed
/// create, and the data path re-creates a NotFound tag on every read — so <c>LibplctagLegacyOrphanReleaseTests</c> fails, so the bump is noticed.</para>
/// </remarks>
internal static class LibplctagOrphanRelease
{
@@ -38,7 +39,10 @@ internal static class LibplctagOrphanRelease
private static int _released;
/// <summary>True when the pinned wrapper's private fields were found, so a failed tag can be destroyed.</summary>
internal static bool CanReadHandles => HandleField is not null;
internal static bool CanReadHandles => HandleField is not null && !ForceRootingForTest;
/// <summary>Test seam (review r1 M1): take the rooting fallback as if the wrapper's fields had been renamed.</summary>
internal static bool ForceRootingForTest { get; set; }
/// <summary>Orphaned native tags destroyed so far (process-wide; diagnostics and tests).</summary>
internal static int Released => Volatile.Read(ref _released);
@@ -50,6 +54,12 @@ internal static class LibplctagOrphanRelease
internal static int HandleOf(Tag tag) =>
HandleField is not null && WrapperField!.GetValue(tag) is { } wrapper ? (int)HandleField.GetValue(wrapper)! : 0;
/// <summary>Clears the wrapper's handle after a destroy, so a repeat release is a no-op (review r1 M2).</summary>
private static void ClearHandle(Tag tag)
{
if (HandleField is not null && WrapperField!.GetValue(tag) is { } wrapper) HandleField.SetValue(wrapper, 0);
}
/// <summary>
/// Runs a libplctag operation that may initialise <paramref name="tag"/> (InitializeAsync, or a first
/// Read/Write, which initialise on demand). If it fails and leaves the tag uninitialised, the native tag it
@@ -103,6 +113,7 @@ internal static class LibplctagOrphanRelease
var status = (Status)plctag.plc_tag_status(handle);
plctag.plc_tag_unregister_callback(handle);
plctag.plc_tag_destroy(handle);
ClearHandle(tag);
Interlocked.Increment(ref _released);
return status;
}
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/LibplctagOrphanReleaseTests.cs
+41
View File
@@ -14,8 +14,20 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests;
/// <c>AbCip.IntegrationTests/LibplctagOrphanReleaseLiveTests</c>; this offline one is the refused-connection
/// variant, whose native tag otherwise keeps reconnecting in the background for the life of the process.
/// </summary>
[CollectionDefinition(Name, DisableParallelization = true)]
public sealed class LibplctagOrphanReleaseCollection
{
/// <summary>The release counters and the rooting seam are process-wide: nothing else may run alongside.</summary>
public const string Name = "libplctag orphan release";
}
[Collection(LibplctagOrphanReleaseCollection.Name)]
public sealed class LibplctagOrphanReleaseTests
{
private static AbCipTagCreateParams Refused(string tagName = "TestDINT") => new(
Gateway: "127.0.0.1", Port: 1, CipPath: "1,0", LibplctagPlcAttribute: "controllogix",
TagName: tagName, Timeout: TimeSpan.FromMilliseconds(300));
[Fact]
public void The_pinned_libplctag_wrapper_still_exposes_the_native_handle()
{
@@ -39,6 +51,8 @@ public sealed class LibplctagOrphanReleaseTests
NativeTagExists(handle).ShouldBeFalse("the failed tag's native handle must be destroyed, not orphaned");
// The status the tag held is kept, not replaced by the destroyed handle's NotFound.
rt.GetStatus().ShouldNotBe((int)AbStatus.NotFound);
// Review r1 M2: the wrapper no longer holds the destroyed id, so a repeat release cannot touch it.
rt.WrapperHandle.ShouldBe(0);
}
finally
{
@@ -46,6 +60,33 @@ public sealed class LibplctagOrphanReleaseTests
}
}
/// <summary>
/// Review r1 M1: when the handle cannot be read (a renamed wrapper field), the failed tag is ROOTED — its callback
/// delegate can then never be collected — and nothing is destroyed.
/// </summary>
[Fact]
public async Task Without_a_readable_handle_the_failed_tag_is_rooted_instead()
{
var rooted = LibplctagOrphanRelease.RootedCount;
var released = LibplctagOrphanRelease.Released;
var rt = new LibplctagTagRuntime(Refused());
LibplctagOrphanRelease.ForceRootingForTest = true;
try
{
await Should.ThrowAsync<AbTransportException>(() => rt.InitializeAsync(CancellationToken.None));
LibplctagOrphanRelease.RootedCount.ShouldBe(rooted + 1);
LibplctagOrphanRelease.Released.ShouldBe(released);
}
finally
{
LibplctagOrphanRelease.ForceRootingForTest = false;
// Clean up the native tag this test deliberately leaked (its callback was unregistered by the timeout).
var handle = rt.NativeHandle;
if (handle > 0) plctag.plc_tag_destroy(handle);
rt.Dispose();
}
}
/// <summary>plc_tag_lock answers PLCTAG_ERR_NOT_FOUND for a handle the library no longer holds.</summary>
internal static bool NativeTagExists(int handle)
{
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/LibplctagLegacyOrphanReleaseTests.cs
+1
View File
@@ -31,6 +31,7 @@ public sealed class LibplctagLegacyOrphanReleaseTests
if (exists) plctag.plc_tag_unlock(handle);
exists.ShouldBeFalse("the failed tag's native handle must be destroyed, not orphaned");
rt.GetStatus().ShouldNotBe((int)AbStatus.NotFound);
rt.WrapperHandle.ShouldBe(0); // review r1 M2: a repeat release cannot touch the destroyed id
}
finally
{