System.Runtime.InteropServices.SafeHandle.InternalRelease Method errors with ObjectDisposedException on MacOS

[cschuchardt88]

Description

When running test(s) on MacOS github action host. I keep getting an ObjectDisposedException than the test host process crashes. For the class SafeHandle on method InternalRelease from one of my tests where I use SafeHandle. Looks like System.GC.RunFinalizers is calling SafeHandle.InternalRelease` method that is causing the issue.

Reproduction Steps

Use this github repo https://github.com/neo-project/neo/tree/7d2a2ad4dc15db7cddefab7fcc2d892561b10216

Expected behavior

Work the same as it does on Windows.

Actual behavior

2025-02-18T19:19:37.3907020Z Test run for /Users/runner/work/neo/neo/tests/Neo.Build.Core.Tests/bin/Debug/net9.0/Neo.Build.Core.Tests.dll (.NETCoreApp,Version=v9.0)
2025-02-18T19:19:37.4610660Z VSTest version 17.12.0 (arm64)
2025-02-18T19:19:37.4616490Z 
2025-02-18T19:19:37.5087550Z Starting test execution, please wait...
2025-02-18T19:19:37.5211390Z A total of 1 test files matched the specified pattern.
2025-02-18T19:19:45.3053130Z The active test run was aborted. Reason: Test host process crashed : Unhandled exception. System.ObjectDisposedException: Cannot access a disposed object.
2025-02-18T19:19:45.3054680Z Object name: 'Neo.Build.Core.Security.UnmanagedBuffer'.
2025-02-18T19:19:45.3288240Z    at System.Runtime.InteropServices.SafeHandle.InternalRelease(Boolean disposeOrFinalizeOperation)
2025-02-18T19:19:45.3397230Z    at System.Runtime.InteropServices.SafeHandle.Finalize()
2025-02-18T19:19:45.3561040Z    at System.GC.RunFinalizers()

Regression?

unknown

Known Workarounds

Unknown

Configuration

https://productionresultssa9.blob.core.windows.net/actions-results/2a93e2b9-6b96-4204-b088-3a65e63d45f2/workflow-job-run-60607ce0-c0fe-5d7b-1e49-b0c53caa438e/logs/job/job-logs.txt?rsct=text%2Fplain&se=2025-02-19T01%3A39%3A48Z&sig=xevB0uDXZPvZUhVgR2fMYh%2FfeiW0GEeTl2TWpSiuX0A%3D&ske=2025-02-19T11%3A55%3A01Z&skoid=ca7593d4-ee42-46cd-af88-8b886a2f84eb&sks=b&skt=2025-02-18T23%3A55%3A01Z&sktid=398a6654-997b-47e9-b12b-9515b896b4de&skv=2025-01-05&sp=r&spr=https&sr=b&st=2025-02-19T01%3A29%3A43Z&sv=2025-01-05

Other information

These are the files that are using SafeHandle
https://github.com/neo-project/neo/blob/7d2a2ad4dc15db7cddefab7fcc2d892561b10216/src/Shared/Neo.Build.Core/Security/UnmanagedBuffer.cs
https://github.com/neo-project/neo/blob/7d2a2ad4dc15db7cddefab7fcc2d892561b10216/src/Shared/Neo.Build.Core/Security/SecureBuffer.cs
https://github.com/neo-project/neo/blob/7d2a2ad4dc15db7cddefab7fcc2d892561b10216/src/Shared/Neo.Build.Core/Security/SecureBuffer.Security.cs

Here are the tests for my above classes
https://github.com/neo-project/neo/blob/7d2a2ad4dc15db7cddefab7fcc2d892561b10216/tests/Neo.Build.Core.Tests/Security/UT_SecureBuffer.cs

[huoyaoyuan]

It means DangerousAddRef and DangerousRelease are unbalanced. Simple repro:

static void M() // Wrapper method to avoid lifetime extending in Debug
{
    var handle = new MyHandle();
    handle.DangerousRelease();
}

M();

GC.Collect();
GC.WaitForPendingFinalizers();

internal class MyHandle() : SafeBuffer(true)
{
    protected override bool ReleaseHandle() => true;
}

[huoyaoyuan]

Also kindly reminder that SafeBuffer is an interop-focused type. For pure managed memory access of unmanaged memory, MemoryManager<T> is more optimal.

[AaronRobinsonMSFT]

@cschuchardt88 As @huoyaoyuan notes this is most likely caused by an imbalance of addref/release calls. With a cursory look at the links in question there is a fair bit of architecture in place that makes understanding what invariants/expectations exist difficult to determine. It is typically beyond the scope of the runtime team to pull down repos and investigate these sorts of issues.

What have you tried to do to narrow the problem space? Have you tried running this on Debug/Release on all platforms? What sort of stress testing exists with these code paths? As previously stated the SafeHandle pattern is generally for interop scenarios and is considered an advanced solution that can be difficult to get correct. Not saying there isn't a bug here, but from the above description it looks like an incorrect use of SafeHandle as opposed to something wrong with the runtime.

The finalizer performing the release is expected, but that is predicated on a correct balance of refs. See below.

runtime/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/SafeHandle.cs

Lines 225 to 229 in 48b566c

	// We should never see a ref count of zero (that would imply we have
	// unbalanced AddRef and Releases). (We might see a closed state before
	// hitting zero though -- that can happen if SetHandleAsInvalid is
	// used).
	ObjectDisposedException.ThrowIf((oldState & StateBits.RefCount) == 0, this);

[cschuchardt88]

I have built a class that does about the same thing as SecureString does (even same code just about). The only difference is it uses encryption to encrypt the memory region on linux and macos (at least the goal). It works fine on local tests on my computer (on Windows). But seems to keep crashing VSTest on Operating Systems (linux, macos) on Github actions with that exception I reported. It seems that the GC is calling RunFinalizers method that is causing the crash of VSTest with ObjectDisposedException on SafeHandle.InternalRelease. On my local computer I use MsTestRunner and does not crash.

[huoyaoyuan]

I have built a class that does about the same thing as SecureString does (even same code just about).

Also note that SecureString is obsolete (See #30612 and #61426) and shouldn't be followed. Processing the bytes in managed array loses the security totally. It's a false sense of security.

The only difference is it uses encryption to encrypt the memory region on linux and macos (at least the goal).

It suggests the issue to be in the encryption code of linux and macos.

[cschuchardt88]

From what I read in those issues, there is no solution? Any recommendations for ways to use Trusted Platform Module (TPM) to encryption a region of memory? Our project is a node for the neo blockchain and the node needs to be able to clear out memory or encrypt it's private keys. Now I understand that the GC makes copies of objects and or moves them around after usage before the memory region is released or reused. Any solutions would be great with using dotnet natively.

[huoyaoyuan]

The core issue is about using the memory securely. It should be an unmanaged Memory<byte> and always accessed directly as Span<byte>, not copied to any managed array. Then you can use any cryptographic API or P/invoke to operate directly on the span.