CurtHagenlocher commented on code in PR #35810:
URL: https://github.com/apache/arrow/pull/35810#discussion_r1210389059
##########
csharp/src/Apache.Arrow/C/CArrowArrayExporter.cs:
##########
@@ -203,6 +207,10 @@ private unsafe static void ReleaseArray(CArrowArray*
cArray)
private unsafe static void Dispose(void** ptr)
{
GCHandle gch = GCHandle.FromIntPtr((IntPtr)(*ptr));
+ if (!gch.IsAllocated)
Review Comment:
This is effectively a noop. If the pointer was null, the previous line will
throw InvalidOperationException and if it wasn't null then IsAllocated will
return true.
The overall change here also means that calling ReleaseArray twice will now
throw an exception instead of the second call being a no-op.
##########
csharp/src/Apache.Arrow/C/CArrowArrayStreamExporter.cs:
##########
@@ -140,6 +167,18 @@ sealed unsafe class ExportedArrayStream : IDisposable
return (void*)GCHandle.ToIntPtr(gch);
}
+ public static void Free(void** ptr)
+ {
+ GCHandle gch = GCHandle.FromIntPtr((IntPtr)ptr);
+ if (!gch.IsAllocated)
Review Comment:
(Same comment about IsAllocated.)
##########
csharp/src/Apache.Arrow/C/CArrowArrayExporter.cs:
##########
@@ -184,13 +189,12 @@ private unsafe static void
ConvertRecordBatch(ExportedAllocationOwner sharedOwne
cArray->dictionary = null;
}
+#if NET5_0_OR_GREATER
+ [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })]
Review Comment:
If there were some reason for the fields to stay public, this one could also
probably be defined as a union between a public IntPtr and a private delegate.
(I don't know why the fields would need to be public; I'm pretty sure I just
followed the pattern that was present for schemas.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
