Skip to content

Commit

Permalink
apacheGH-40898: [C#] Do not import length-zero buffers from C Data In…
Browse files Browse the repository at this point in the history 
…terface Arrays (apache#41054)

### Rationale for this change

When implementing integration tests for nanoarrow, it was observed that C# never released arrays where `array->buffers[i]` was `NULL` (including any buffers of any recursive child arrays). This is allowed ( https://arrow.apache.org/docs/format/CDataInterface.html#c.ArrowArray.buffers ); however, every other implementation appears to allocate even for length zero buffers (including nanoarrow after apache/arrow-nanoarrow#399 ).

### What changes are included in this PR?

`AddMemory()` is replaced with `ArrowBuffer.Empty` if the length of the imported buffer would have been 0 bytes. For other buffers (or anywhere I saw dereferencing a buffer pointer), I added a `Debug.Assert` just to be sure.

### Are these changes tested?

I'm not sure what the best way to test them is! They won't be tested in the nanoarrow integration tests since at the point that they run, nanoarrow will no longer export arrays that would trigger this.

### Are there any user-facing changes?

No
* GitHub Issue: apache#40898

Authored-by: Dewey Dunnington <dewey@fishandwhistle.net>
Signed-off-by: Curt Hagenlocher <curt@hagenlocher.org>
  • Loading branch information
@paleolimbot @vibhatha
paleolimbot authored and vibhatha committed May 25, 2024
1 parent 11918b4 commit e5b3560
Showing 1 changed file with 28 additions and 13 deletions.
41 changes: 28 additions & 13 deletions csharp/src/Apache.Arrow/C/CArrowArrayImporter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.InteropServices;
using Apache.Arrow.Memory;
using Apache.Arrow.Types;
Expand All @@ -36,7 +37,7 @@ public static class CArrowArrayImporter
/// Typically, you will allocate an uninitialized CArrowArray pointer,
/// pass that to external function, and then use this method to import
/// the result.
///
///
/// <code>
/// CArrowArray* importedPtr = CArrowArray.Create();
/// foreign_export_function(importedPtr);
Expand Down Expand Up @@ -71,7 +72,7 @@ public static unsafe IArrowArray ImportArray(CArrowArray* ptr, IArrowType type)
/// Typically, you will allocate an uninitialized CArrowArray pointer,
/// pass that to external function, and then use this method to import
/// the result.
///
///
/// <code>
/// CArrowArray* importedPtr = CArrowArray.Create();
/// foreign_export_function(importedPtr);
Expand Down Expand Up @@ -256,6 +257,19 @@ private ArrowBuffer ImportValidityBuffer(CArrowArray* cArray)
return (cArray->buffers[0] == null) ? ArrowBuffer.Empty : new ArrowBuffer(AddMemory((IntPtr)cArray->buffers[0], 0, validityLength));
}

private ArrowBuffer ImportCArrayBuffer(CArrowArray* cArray, int i, int lengthBytes)
{
if (lengthBytes > 0)
{
Debug.Assert(cArray->buffers[i] != null);
return new ArrowBuffer(AddMemory((IntPtr)cArray->buffers[i], 0, lengthBytes));
}
else
{
return ArrowBuffer.Empty;
}
}

private ArrowBuffer[] ImportByteArrayBuffers(CArrowArray* cArray)
{
if (cArray->n_buffers != 3)
Expand All @@ -266,12 +280,13 @@ private ArrowBuffer[] ImportByteArrayBuffers(CArrowArray* cArray)
int length = checked((int)cArray->length);
int offsetsLength = (length + 1) * 4;
int* offsets = (int*)cArray->buffers[1];
Debug.Assert(offsets != null);
int valuesLength = offsets[length];

ArrowBuffer[] buffers = new ArrowBuffer[3];
buffers[0] = ImportValidityBuffer(cArray);
buffers[1] = new ArrowBuffer(AddMemory((IntPtr)cArray->buffers[1], 0, offsetsLength));
buffers[2] = new ArrowBuffer(AddMemory((IntPtr)cArray->buffers[2], 0, valuesLength));
buffers[1] = ImportCArrayBuffer(cArray, 1, offsetsLength);
buffers[2] = ImportCArrayBuffer(cArray, 2, valuesLength);

return buffers;
}
Expand All @@ -289,10 +304,10 @@ private ArrowBuffer[] ImportByteArrayViewBuffers(CArrowArray* cArray)
long* bufferLengths = (long*)cArray->buffers[cArray->n_buffers - 1];
ArrowBuffer[] buffers = new ArrowBuffer[cArray->n_buffers - 1];
buffers[0] = ImportValidityBuffer(cArray);
buffers[1] = new ArrowBuffer(AddMemory((IntPtr)cArray->buffers[1], 0, viewsLength));
buffers[1] = ImportCArrayBuffer(cArray, 1, viewsLength);
for (int i = 2; i < buffers.Length; i++)
{
buffers[i] = new ArrowBuffer(AddMemory((IntPtr)cArray->buffers[i], 0, checked((int)bufferLengths[i - 2])));
buffers[i] = ImportCArrayBuffer(cArray, i, checked((int)bufferLengths[i - 2]));
}

return buffers;
Expand All @@ -310,7 +325,7 @@ private ArrowBuffer[] ImportListBuffers(CArrowArray* cArray)

ArrowBuffer[] buffers = new ArrowBuffer[2];
buffers[0] = ImportValidityBuffer(cArray);
buffers[1] = new ArrowBuffer(AddMemory((IntPtr)cArray->buffers[1], 0, offsetsLength));
buffers[1] = ImportCArrayBuffer(cArray, 1, offsetsLength);

return buffers;
}
Expand All @@ -327,8 +342,8 @@ private ArrowBuffer[] ImportListViewBuffers(CArrowArray* cArray)

ArrowBuffer[] buffers = new ArrowBuffer[3];
buffers[0] = ImportValidityBuffer(cArray);
buffers[1] = new ArrowBuffer(AddMemory((IntPtr)cArray->buffers[1], 0, offsetsLength));
buffers[2] = new ArrowBuffer(AddMemory((IntPtr)cArray->buffers[2], 0, offsetsLength));
buffers[1] = ImportCArrayBuffer(cArray, 1, offsetsLength);
buffers[2] = ImportCArrayBuffer(cArray, 2, offsetsLength);

return buffers;
}
Expand Down Expand Up @@ -356,8 +371,8 @@ private ArrowBuffer[] ImportDenseUnionBuffers(CArrowArray* cArray)
int offsetsLength = length * 4;

ArrowBuffer[] buffers = new ArrowBuffer[2];
buffers[0] = new ArrowBuffer(AddMemory((IntPtr)cArray->buffers[0], 0, length));
buffers[1] = new ArrowBuffer(AddMemory((IntPtr)cArray->buffers[1], 0, offsetsLength));
buffers[0] = ImportCArrayBuffer(cArray, 0, length);
buffers[1] = ImportCArrayBuffer(cArray, 1, offsetsLength);

return buffers;
}
Expand All @@ -370,7 +385,7 @@ private ArrowBuffer[] ImportSparseUnionBuffers(CArrowArray* cArray)
}

ArrowBuffer[] buffers = new ArrowBuffer[1];
buffers[0] = new ArrowBuffer(AddMemory((IntPtr)cArray->buffers[0], 0, checked((int)cArray->length)));
buffers[0] = ImportCArrayBuffer(cArray, 0, checked((int)cArray->length));

return buffers;
}
Expand All @@ -392,7 +407,7 @@ private ArrowBuffer[] ImportFixedWidthBuffers(CArrowArray* cArray, int bitWidth)

ArrowBuffer[] buffers = new ArrowBuffer[2];
buffers[0] = ImportValidityBuffer(cArray);
buffers[1] = new ArrowBuffer(AddMemory((IntPtr)cArray->buffers[1], 0, valuesLength));
buffers[1] = ImportCArrayBuffer(cArray, 1, valuesLength);

return buffers;
}
Expand Down

0 comments on commit e5b3560

Please sign in to comment.