Ensure that converted objects are pinned to avoid invalid accesses due to memory movement

[Braedon-Wooding-Displayr]

We can't trust gc_root since it doesn't pin the memory so we need to use pin_ptr or gc handle. I've opted for gc handle here for no specific reason.

[Braedon-Wooding-Displayr]

gc roots don't pin the memory so we can't hold them as ptrs.

This is only really safe right now is because memory only moves for long lived memory (short lived memory never moves) and all these converted objects fall under short lived memory.

[oliverbock]

Looks like a good change, though it's been a long time since I've used any of this stuff so I might be wrong. Perhaps check with Brandon too.

[bwinsley]

LGTM

[spahnke]

Disclaimer: I don't work much with C++/CLR, gcroot and/or GCHandle, or native interop in general, so forgive and correct me if I'm wrong here.

This change looks more unsafe to me and in fact the previous code seems completely fine. From what I understand (and you can see that by inspecting the header file), a gcroot is just a wrapper around a GCHandle that automatically calls Alloc (with GCHandleType::Normal implicitly) and Free when it is created/deleted. What we had before (and even before the change in a784f1d) was a native heap-allocated gcroot<Object^> * in both the V8 external slot and (now) the C++ vector. The gcroot/GCHandle ensures that the underlying CLR object is not garbage collected. The native pointer to the gcroot remains stable at all times including reallocations/moves of the vector storage itself when push_back is called, but also moving of the CLR object by the garbage collector which is completely opaque to us because we don't hold a pointer to the managed memory itself. This allows us to put this stable pointer into the V8 external slot which requires a pointer type. When we then derefence the gcroot<Object^> * in GetConverted we just get back a managed Object^ handle to the CLR object and it doesn't matter if the underlying memory was moved by the GC. When we delete the gcroot<Object^> * in ~ConvertedObjects the destructor of the gcroot just frees the GCHandle for us and makes the CLR object garbage collectable again. So from what I gather we don't even need the managed memory to be pinned for any of the operations here, correct?

This PR now basically does what gcroot does manually, still uses GCHandleType::Normal (explicitly this time) so doesn't pin the managed memory, but then also calls GCHandle::ToIntPtr(handle).ToPointer() to put into the vector and the V8 external slot, which as far as I can tell returns a pointer to the actual underlying managed memory which, if it moves, now actually invalidates this pointer. Am I missing something here? Am I misunderstanding the documentation of ToPointer() (https://learn.microsoft.com/en-us/dotnet/api/system.intptr.topointer?view=net-10.0)?