Consider Implementing IDisposable in GCHandle #29528
Comments
|
(I am happy to open a PR incl. tests if this is approved) |
|
This has been discussed for quite some time and part of the concern around doing this is it encourages developers to box the If we pretended for a moment that var obj = new object();
var handle = GCHandle.Alloc(obj);
Console.WriteLine(handle.IsAllocated);
((IDisposable)handle).Dispose(); //Box and dispose
Console.WriteLine(handle.IsAllocated);The box and dispose here would duplicate the struct and |
|
Oh LOL. Actually forgot the fact that it's a struct. silently closes the issue |
Does that really matter? Its use is still guarded and this has same problem var obj = new object();
var handle = GCHandle.Alloc(obj);
Console.WriteLine(handle.IsAllocated);
DoSomethingWithHandle(handle); //Pass by value and dispose
Console.WriteLine(handle.IsAllocated);
void DoSomethingWithHandle(GCHandle handle)
{
handle.Free();
} |
|
Maybe I'll just let the people here to close it. We'll see what kind of response we'll get |
|
While the boxing isn't ideal, the value type nature of |
|
You can dupe the struct by setting it to another variable or pass by value; or set it to a |
I'm not sure I follow.
If you free a copy of it then I agree it is fairly easy to cause a |
The one that thinks its still allocated will throw an InvalidOperation "This handle is not pinned"; it checks with the GC before giving you the address:
Its use it guarded, regardless of what state the struct thinks its in |
Maybe that is new for 3 then? Here is what I get with .NET Core 2.2: static void Main(string[] args)
{
var bytes = new byte[] { 1 };
var obj = GCHandle.Alloc(bytes, GCHandleType.Pinned);
DumpAndFree(obj);
DumpAndFree(obj);
void DumpAndFree(GCHandle handle)
{
var ptr = handle.AddrOfPinnedObject();
Console.WriteLine(Marshal.ReadByte(ptr));
handle.Free();
}
} |
|
Ah, in your screenshot your |
|
Ah... my mistake; I thought it did an additional check, but that's only in debug #if DEBUG
// The runtime performs additional checks in debug builds
[MethodImpl(MethodImplOptions.InternalCall)]
internal static extern object InternalGet(IntPtr handle);
#else
internal static unsafe object InternalGet(IntPtr handle) =>
Unsafe.As<IntPtr, object>(ref *(IntPtr*)handle);
#endif |
|
Anyway, given that:
Perhaps it isn't too bad of an idea to implement. |
It would be very bad for struct things if it did start boxing them. However it would be important not to be explicitly implemented; as that would only being downsides, other than the slightly nicer syntax. |
|
You can still get in to trouble with GCHandle handle = // Get a handle
using (handle)
{
}
// handle would still be IsAllocated=true hereWhich is admittedly a little weird. It seems |
|
😱 TIL That's kinda weird? static void Main(string[] args)
{
GCHandlish handle = GCHandlish.Create();
using (handle)
{
Console.WriteLine(handle.IsAllocated);
}
Console.WriteLine(handle.IsAllocated);
}
struct GCHandlish : IDisposable
{
public bool IsAllocated { get; private set; }
public static GCHandlish Create() => new GCHandlish() { IsAllocated = true };
public void Dispose() => IsAllocated = false;
} |
|
Is fine if you create it in the static void Main(string[] args)
{
using (GCHandlish handle = GCHandlish.Create())
{
Console.WriteLine(handle.IsAllocated);
}
} |
maryamariyan
added
the
untriaged
jeffschwMSFT
removed
the
untriaged
|
Closing this as a duplicate of #54792. This one should have been used but was missed. A lot of conversation has occurred on the new issue with a referenced PR. |
ghost
locked as resolved and limited conversation to collaborators
(Pretty much duplicate of #26854, re-opening due to reason that is to be explained)
GCHandle is supposed to be freed once it goes out of scope: otherwise, memory leaks may occur. Although it has a
Free()method which does that, it does not implementIDisposableandDispose()method which lets it to be used inusingblocks. Having it would be helpful since major .NET Languages such as C#, VB.NET, and F# provides syntactic sugar for using disposable types,using/use. I believe it's a pretty common practice for disposing of types that hold unmanaged resources.The original issue, #26854, discusses about the exact same suggestion; however, the language feature proposal appear to have changed not to include this change for non-ref struct types, due to backwards-compat concerns.
This leaves the original problem unsolved. For that reason, I suggest that we reconsider implementing the interface on the type.
The text was updated successfully, but these errors were encountered: