Fix double dispose of GCHandle in BrowserWebSocket

[filipnavara]

MemoryHandle is a struct, so passing it into an IDisposable parameter creates a box. This causes two copies of GCHandle to exist which are no longer synchronized and can be double disposed.

This happened when passing pinBuffer into CancellationHelper in ReceiveAsyncCore and SendAsyncCore. In both cases the caller does the Dispose itself inside a finally, so there's no need to pass the pinBuffer to CancellationHelper at all.

[filipnavara]

This has been reported downstream in NativeAOT-LLVM where the double-free of the GCHandle causes an assert (in Debug) or a corruption. (/cc @yowl for the original report)

[dotnet-policy-service]

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

[EgorBo]

A minimal repro for the issue Filip was tracing:

using System.Buffers;

ThreadPool.QueueUserWorkItem(_ => GC.Collect());

for (int i = 0; i < 100; i++)
{
    Memory<byte> bufferMemory = new byte[100];
    MemoryHandle mh = bufferMemory.Pin();
    IDisposable d = mh;
    d.Dispose();
    mh.Dispose();
}

Fails for me on the Checked runtime with:

Assert failure(PID 281072 [0x000449f0], Thread: 376552 [0x5bee8]): FALSE

CORECLR! BlockFreeHandles + 0x158 (0x00007ff8`6dbdff68)
CORECLR! TableFreeBulkPreparedHandles + 0x112 (0x00007ff8`6dbe1812)
CORECLR! TableFullRebalanceCache + 0x1C8 (0x00007ff8`6dc79f98)
CORECLR! TableFreeSingleHandleToCache + 0x178 (0x00007ff8`6dc79d58)
CORECLR! HndDestroyHandle + 0x222 (0x00007ff8`6dbdf1e2)
CORECLR! HndDestroyHandleOfUnknownType + 0xE1 (0x00007ff8`6dbdf321)
CORECLR! MarshalNative::GCHandleInternalFree + 0x126 (0x00007ff8`6d9915f6)
SYSTEM.PRIVATE.CORELIB! <no symbol> + 0x0 (0x00007ff8`6c41127f)
SYSTEM.PRIVATE.CORELIB! <no symbol> + 0x0 (0x00007ff8`6c379dab)
<no module>! <no symbol> + 0x0 (0x00007ff8`0e441e5c)
    File: C:\prj\runtime-main2\src\coreclr\gc\handletablecore.cpp:1899
    Image: C:\prj\runtime-main2\artifacts\bin\coreclr\windows.x64.Checked\CoreRun.exe

When we clone MemoryHandle (via boxing) and call Dispose on both the original struct and its boxed clone, we end up calling GCHandle.Free and IPinable.Unpin twice here.

@jkotas is it something that shouldn't happen or just an UB on double-free?

[huoyaoyuan]

Double free of GCHandle is faulty. See also #112727

[jkotas]

is it something that shouldn't happen or just an UB on double-free?

It is just an UB on double-free.

[pavelsavara]

This will need 9.0 backport. Net8 doesn't have this issue.

[pavelsavara]

/ba-g know CI timeouts

[pavelsavara]

/backport to release/9.0-staging

[github-actions]

Started backporting to release/9.0-staging: https://github.com/dotnet/runtime/actions/runs/13860807379