diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagOrphanRelease.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagOrphanRelease.cs index 11b756a9..f0defa6e 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagOrphanRelease.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagOrphanRelease.cs @@ -31,7 +31,11 @@ internal static class LibplctagOrphanRelease private const BindingFlags Private = BindingFlags.NonPublic | BindingFlags.Instance; private static readonly FieldInfo? WrapperField = typeof(Tag).GetField("_tag", Private); - private static readonly FieldInfo? HandleField = WrapperField?.FieldType.GetField("nativeTagHandle", Private); + // Final review M2: the field must also still be an int. A wrapper that retyped it (IntPtr, long) would make + // the cast in HandleOf throw; treating it as unreadable instead degrades to the rooting fallback ("keep + // alive"), which the pin test reports. + private static readonly FieldInfo? HandleField = + WrapperField?.FieldType.GetField("nativeTagHandle", Private) is { } f && f.FieldType == typeof(int) ? f : null; /// Tags whose handle could not be read: kept reachable so their callback delegate is never collected. private static readonly ConcurrentBag Rooted = []; @@ -50,9 +54,20 @@ internal static class LibplctagOrphanRelease /// Orphaned tags rooted for the process lifetime because their handle could not be read. internal static int RootedCount => Rooted.Count; - /// The native handle the wrapper holds (0 when none was ever created, or unreadable). - internal static int HandleOf(Tag tag) => - HandleField is not null && WrapperField!.GetValue(tag) is { } wrapper ? (int)HandleField.GetValue(wrapper)! : 0; + /// The native handle the wrapper holds (0 when none was ever created, or unreadable). Never throws: + /// callers run it inside a catch block, where an exception would replace the failure being handled. + internal static int HandleOf(Tag tag) + { + try + { + return HandleField is not null && WrapperField!.GetValue(tag) is { } wrapper + && HandleField.GetValue(wrapper) is int handle ? handle : 0; + } + catch (Exception) + { + return 0; + } + } /// Clears the wrapper's handle after a destroy, so a repeat release is a no-op (review r1 M2). private static void ClearHandle(Tag tag) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagOrphanRelease.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagOrphanRelease.cs index ffaf5274..7b4ec909 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagOrphanRelease.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagOrphanRelease.cs @@ -31,7 +31,11 @@ internal static class LibplctagOrphanRelease private const BindingFlags Private = BindingFlags.NonPublic | BindingFlags.Instance; private static readonly FieldInfo? WrapperField = typeof(Tag).GetField("_tag", Private); - private static readonly FieldInfo? HandleField = WrapperField?.FieldType.GetField("nativeTagHandle", Private); + // Final review M2: the field must also still be an int. A wrapper that retyped it (IntPtr, long) would make + // the cast in HandleOf throw; treating it as unreadable instead degrades to the rooting fallback ("keep + // alive"), which the pin test reports. + private static readonly FieldInfo? HandleField = + WrapperField?.FieldType.GetField("nativeTagHandle", Private) is { } f && f.FieldType == typeof(int) ? f : null; /// Tags whose handle could not be read: kept reachable so their callback delegate is never collected. private static readonly ConcurrentBag Rooted = []; @@ -50,9 +54,20 @@ internal static class LibplctagOrphanRelease /// Orphaned tags rooted for the process lifetime because their handle could not be read. internal static int RootedCount => Rooted.Count; - /// The native handle the wrapper holds (0 when none was ever created, or unreadable). - internal static int HandleOf(Tag tag) => - HandleField is not null && WrapperField!.GetValue(tag) is { } wrapper ? (int)HandleField.GetValue(wrapper)! : 0; + /// The native handle the wrapper holds (0 when none was ever created, or unreadable). Never throws: + /// callers run it inside a catch block, where an exception would replace the failure being handled. + internal static int HandleOf(Tag tag) + { + try + { + return HandleField is not null && WrapperField!.GetValue(tag) is { } wrapper + && HandleField.GetValue(wrapper) is int handle ? handle : 0; + } + catch (Exception) + { + return 0; + } + } /// Clears the wrapper's handle after a destroy, so a repeat release is a no-op (review r1 M2). private static void ClearHandle(Tag tag) diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/LibplctagOrphanReleaseTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/LibplctagOrphanReleaseTests.cs index 55dd52ad..3d1b3a62 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/LibplctagOrphanReleaseTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/LibplctagOrphanReleaseTests.cs @@ -36,6 +36,28 @@ public sealed class LibplctagOrphanReleaseTests LibplctagOrphanRelease.CanReadHandles.ShouldBeTrue(); } + /// + /// Final review M2: the handle field's TYPE is pinned explicitly, not just its presence. A retype (IntPtr, + /// long) now degrades to the rooting fallback instead of an InvalidCastException inside a catch block, + /// and this names the reason when it happens. + /// + [Fact] + public void The_pinned_wrapper_handle_field_is_still_an_int() + { + const System.Reflection.BindingFlags priv = + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance; + var wrapper = typeof(libplctag.Tag).GetField("_tag", priv).ShouldNotBeNull(); + wrapper.FieldType.GetField("nativeTagHandle", priv).ShouldNotBeNull().FieldType.ShouldBe(typeof(int)); + } + + /// HandleOf never throws (it runs inside the runtimes' catch blocks); a never-created tag reads 0. + [Fact] + public void HandleOf_a_fresh_tag_is_zero_and_does_not_throw() + { + using var tag = new libplctag.Tag(); + LibplctagOrphanRelease.HandleOf(tag).ShouldBe(0); + } + [Fact] public async Task A_failed_initialisation_destroys_the_native_tag_it_created() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/LibplctagLegacyOrphanReleaseTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/LibplctagLegacyOrphanReleaseTests.cs index 8b34d284..913eff67 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/LibplctagLegacyOrphanReleaseTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/LibplctagLegacyOrphanReleaseTests.cs @@ -16,6 +16,28 @@ public sealed class LibplctagLegacyOrphanReleaseTests public void The_pinned_libplctag_wrapper_still_exposes_the_native_handle() => LibplctagOrphanRelease.CanReadHandles.ShouldBeTrue(); + /// + /// Final review M2: the handle field's TYPE is pinned explicitly, not just its presence. A retype (IntPtr, + /// long) now degrades to the rooting fallback instead of an InvalidCastException inside a catch block, + /// and this names the reason when it happens. + /// + [Fact] + public void The_pinned_wrapper_handle_field_is_still_an_int() + { + const System.Reflection.BindingFlags priv = + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance; + var wrapper = typeof(libplctag.Tag).GetField("_tag", priv).ShouldNotBeNull(); + wrapper.FieldType.GetField("nativeTagHandle", priv).ShouldNotBeNull().FieldType.ShouldBe(typeof(int)); + } + + /// HandleOf never throws (it runs inside the runtimes' catch blocks); a never-created tag reads 0. + [Fact] + public void HandleOf_a_fresh_tag_is_zero_and_does_not_throw() + { + using var tag = new libplctag.Tag(); + LibplctagOrphanRelease.HandleOf(tag).ShouldBe(0); + } + [Fact] public async Task A_failed_initialisation_destroys_the_native_tag_it_created() {