fix(ab): final review M2 — a retyped libplctag handle field degrades to keep-alive
HandleOf cast the reflected nativeTagHandle with (int) and was called inside the runtimes' catch blocks, so a libplctag bump that retyped the field would replace every create failure with an InvalidCastException. The field is now accepted only if it is still an int (otherwise CanReadHandles is false and orphans are rooted), and HandleOf never throws. Both projects pin the field type explicitly. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
@@ -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;
|
||||
|
||||
/// <summary>Tags whose handle could not be read: kept reachable so their callback delegate is never collected.</summary>
|
||||
private static readonly ConcurrentBag<Tag> Rooted = [];
|
||||
@@ -50,9 +54,20 @@ internal static class LibplctagOrphanRelease
|
||||
/// <summary>Orphaned tags rooted for the process lifetime because their handle could not be read.</summary>
|
||||
internal static int RootedCount => Rooted.Count;
|
||||
|
||||
/// <summary>The native handle the wrapper holds (0 when none was ever created, or unreadable).</summary>
|
||||
internal static int HandleOf(Tag tag) =>
|
||||
HandleField is not null && WrapperField!.GetValue(tag) is { } wrapper ? (int)HandleField.GetValue(wrapper)! : 0;
|
||||
/// <summary>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.</summary>
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
||||
/// <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)
|
||||
|
||||
@@ -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;
|
||||
|
||||
/// <summary>Tags whose handle could not be read: kept reachable so their callback delegate is never collected.</summary>
|
||||
private static readonly ConcurrentBag<Tag> Rooted = [];
|
||||
@@ -50,9 +54,20 @@ internal static class LibplctagOrphanRelease
|
||||
/// <summary>Orphaned tags rooted for the process lifetime because their handle could not be read.</summary>
|
||||
internal static int RootedCount => Rooted.Count;
|
||||
|
||||
/// <summary>The native handle the wrapper holds (0 when none was ever created, or unreadable).</summary>
|
||||
internal static int HandleOf(Tag tag) =>
|
||||
HandleField is not null && WrapperField!.GetValue(tag) is { } wrapper ? (int)HandleField.GetValue(wrapper)! : 0;
|
||||
/// <summary>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.</summary>
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
||||
/// <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)
|
||||
|
||||
@@ -36,6 +36,28 @@ public sealed class LibplctagOrphanReleaseTests
|
||||
LibplctagOrphanRelease.CanReadHandles.ShouldBeTrue();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
[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));
|
||||
}
|
||||
|
||||
/// <summary>HandleOf never throws (it runs inside the runtimes' catch blocks); a never-created tag reads 0.</summary>
|
||||
[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()
|
||||
{
|
||||
|
||||
+22
@@ -16,6 +16,28 @@ public sealed class LibplctagLegacyOrphanReleaseTests
|
||||
public void The_pinned_libplctag_wrapper_still_exposes_the_native_handle() =>
|
||||
LibplctagOrphanRelease.CanReadHandles.ShouldBeTrue();
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
[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));
|
||||
}
|
||||
|
||||
/// <summary>HandleOf never throws (it runs inside the runtimes' catch blocks); a never-created tag reads 0.</summary>
|
||||
[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()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user