fix(ab): destroy the native libplctag tag a failed initialisation leaves behind (T21 press pass 1 crash)

The T21 press pass 1 test host died at exit with "A callback was made on a garbage collected delegate of type ...plctag+callback_func_ex::Invoke". The cause is in libplctag.NET 1.5.2. When a tag's create fails, for example with NotFound, InitializeAsync throws with the callback still registered. Dispose() destroys a tag only if it is initialised, so the native tag outlives its wrapper. The GC then frees the delegate, and libplctag's atexit plc_tag_shutdown raises DESTROYED into it. Reproduced against ab_server: the production LibplctagTagRuntime with a NotFound tag exits 134 with that exact message, and exits 0 with this fix. A timed-out create unregisters its callback but never destroys the handle either, so that native tag keeps reconnecting for the life of the process. The fix adds LibplctagOrphanRelease (AbCip and AbLegacy). After a failed initialise/read/write that leaves the tag uninitialised, it reads the handle through the pinned wrapper's private fields, unregisters the callback and destroys the tag. If those fields cannot be read, it roots the tag for the life of the process instead. It is applied in both tag runtimes, the symbol-listing reader and the template reader. GetStatus keeps the status the tag held before it was destroyed. Pins: offline refused-connection tests in AbCip.Tests and AbLegacy.Tests, a live NotFound test on ab_server, and a reflection-shape pin. All go red with the release disabled. The differential harness also asserts that no orphan had to be rooted. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
2026-10-07 19:49:03 -04:00
parent c306c6f674
commit 6cd6876229
10 changed files with 462 additions and 12 deletions
@@ -138,6 +138,21 @@ public sealed class TransportDifferentialTests(DifferentialRunFixture fixture)
        r.ConnectionsBeforeNative.Value.ShouldBeLessThanOrEqualTo(r.ConnectionsBaseline.Value);
    }

+    /// <summary>
+    ///     T21 press pass 1 ended in "A callback was made on a garbage collected delegate": libplctag tags whose
+    ///     initialisation failed (NotFound tags, refused latency setups) were left native-side with a live callback.
+    ///     Every libplctag tag this harness creates goes through the driver's adapters, which now destroy such a tag
+    ///     (<see cref="LibplctagOrphanRelease"/>); this pins that none had to fall back to being rooted instead.
+    /// </summary>
+    [Fact]
+    public async Task No_failed_libplctag_tag_is_left_with_a_live_native_callback()
+    {
+        await RunOrSkipAsync();
+        Log($"libplctag orphans destroyed so far: {LibplctagOrphanRelease.Released}; rooted (unreleasable): {LibplctagOrphanRelease.RootedCount}.");
+        LibplctagOrphanRelease.CanReadHandles.ShouldBeTrue();
+        LibplctagOrphanRelease.RootedCount.ShouldBe(0);
+    }
+
    [Fact]
    public async Task The_native_side_makes_exactly_one_forward_open_over_the_whole_run()
    {
@@ -0,0 +1,43 @@
+using libplctag.NativeImport;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests;
+
+/// <summary>
+///     T21 press pass 1 crash, the variant that killed the test host: a tag the controller does not have. libplctag
+///     raises <c>Created</c> with NotFound, so libplctag.NET 1.5.2 throws WITHOUT unregistering its callback, and its
+///     <c>Dispose()</c> skips the uninitialised tag. Before the fix the native tag survived with a live callback into a
+///     delegate the GC was free to collect; libplctag's <c>atexit</c> shutdown then raised DESTROYED into it ("A callback
+///     was made on a garbage collected delegate"). This pins that the native tag is destroyed instead.
+/// </summary>
+[Trait("Category", "Integration")]
+[Trait("Requires", "AbServer")]
+public sealed class LibplctagOrphanReleaseLiveTests
+{
+    [AbServerFact]
+    public async Task A_tag_the_controller_lacks_leaves_no_native_tag_behind()
+    {
+        var fixture = new AbServerFixture();
+        var rt = new LibplctagTagRuntime(new AbCipTagCreateParams(
+            Gateway: fixture.Host, Port: fixture.Port, CipPath: "1,0", LibplctagPlcAttribute: "controllogix",
+            TagName: "T21_NoSuchTag", Timeout: TimeSpan.FromSeconds(5)));
+        try
+        {
+            var ex = await Should.ThrowAsync<AbTransportException>(() => rt.InitializeAsync(CancellationToken.None));
+            ex.Status.ShouldBe(AbStatus.NotFound);
+            rt.GetStatus().ShouldBe((int)AbStatus.NotFound);
+
+            var handle = rt.NativeHandle;
+            handle.ShouldBeGreaterThan(0);
+            var exists = plctag.plc_tag_lock(handle) == 0;
+            if (exists) plctag.plc_tag_unlock(handle);
+            exists.ShouldBeFalse("a NotFound tag must not survive with its callback registered");
+        }
+        finally
+        {
+            rt.Dispose();
+        }
+    }
+}
@@ -0,0 +1,56 @@
+using libplctag.NativeImport;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests;
+
+/// <summary>
+///     T21 press pass 1: a libplctag tag whose initialisation FAILED used to leave its native tag behind, because
+///     libplctag.NET 1.5.2's <c>Dispose()</c> only destroys an initialised tag. Its callback delegate then died with
+///     the wrapper, and libplctag's <c>atexit</c> shutdown raised DESTROYED into it: "A callback was made on a garbage
+///     collected delegate". These pin that a failed initialisation destroys the native tag before it propagates.
+///     The NotFound variant (the one that crashed, callback still registered) needs a controller and lives in
+///     <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>
+public sealed class LibplctagOrphanReleaseTests
+{
+    [Fact]
+    public void The_pinned_libplctag_wrapper_still_exposes_the_native_handle()
+    {
+        // A libplctag bump that renames Tag._tag / NativeTagWrapper.nativeTagHandle disables the release (the tag is
+        // then rooted for the process lifetime instead). Fail here so the bump is reviewed against the release.
+        LibplctagOrphanRelease.CanReadHandles.ShouldBeTrue();
+    }
+
+    [Fact]
+    public async Task A_failed_initialisation_destroys_the_native_tag_it_created()
+    {
+        // Port 1 on loopback refuses the connection; libplctag keeps retrying until the timeout.
+        var rt = new LibplctagTagRuntime(new AbCipTagCreateParams(
+            Gateway: "127.0.0.1", Port: 1, CipPath: "1,0", LibplctagPlcAttribute: "controllogix",
+            TagName: "TestDINT", Timeout: TimeSpan.FromMilliseconds(300)));
+        try
+        {
+            await Should.ThrowAsync<AbTransportException>(() => rt.InitializeAsync(CancellationToken.None));
+            var handle = rt.NativeHandle;
+            handle.ShouldBeGreaterThan(0, "plc_tag_create_ex returned a handle before the failure");
+            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);
+        }
+        finally
+        {
+            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)
+    {
+        if (plctag.plc_tag_lock(handle) != 0) return false;
+        plctag.plc_tag_unlock(handle);
+        return true;
+    }
+}
@@ -0,0 +1,40 @@
+using libplctag.NativeImport;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests;
+
+/// <summary>
+///     T21 press pass 1, AbLegacy side: a failed libplctag initialisation must destroy the native tag it created
+///     (libplctag.NET 1.5.2's <c>Dispose()</c> skips an uninitialised tag, leaving a native tag whose callback can fire
+///     into a collected delegate and kill the process). See <c>LibplctagOrphanRelease</c>.
+/// </summary>
+public sealed class LibplctagLegacyOrphanReleaseTests
+{
+    [Fact]
+    public void The_pinned_libplctag_wrapper_still_exposes_the_native_handle() =>
+        LibplctagOrphanRelease.CanReadHandles.ShouldBeTrue();
+
+    [Fact]
+    public async Task A_failed_initialisation_destroys_the_native_tag_it_created()
+    {
+        var rt = new LibplctagLegacyTagRuntime(new AbLegacyTagCreateParams(
+            Gateway: "127.0.0.1", Port: 1, CipPath: "1,0", LibplctagPlcAttribute: "slc500",
+            TagName: "N7:0", Timeout: TimeSpan.FromMilliseconds(300)));
+        try
+        {
+            await Should.ThrowAsync<AbTransportException>(() => rt.InitializeAsync(CancellationToken.None));
+            var handle = rt.NativeHandle;
+            handle.ShouldBeGreaterThan(0, "plc_tag_create_ex returned a handle before the failure");
+            var exists = plctag.plc_tag_lock(handle) == 0;
+            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);
+        }
+        finally
+        {
+            rt.Dispose();
+        }
+    }
+}