CurtHagenlocher opened a new issue, #397:
URL: https://github.com/apache/arrow-dotnet/issues/397
### Summary
A `ReadOnlySpan<byte>` obtained from `ArrowBuffer.Span` does not keep the
buffer's memory alive. Because `MemoryAllocator.Default` allocates native
memory owned by a `NativeMemoryManager` **whose finalizer frees it**, an array
that becomes unreachable while spans over its buffers are still in use can have
those buffers freed underneath them.
The result is a use-after-free in ordinary-looking consumer code. The
failure is usually silent — freed `HGlobal` pages typically stay mapped, so the
span keeps returning plausible data — and only occasionally surfaces as an
`AccessViolationException`.
### Reproduction
`repro.csproj`:
```xml
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net8.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Apache.Arrow" Version="23.0.0" />
</ItemGroup>
</Project>
```
`Program.cs`:
```csharp
using System.Buffers;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using Apache.Arrow;
static StringArray BuildArray(int rows)
{
var b = new StringArray.Builder();
for (int i = 0; i < rows; i++) b.Append($"row-{i}");
return b.Build();
}
// WeakReference to the MemoryManager that owns the array's value buffer.
static WeakReference OwnerOf(StringArray a)
{
MemoryMarshal.TryGetMemoryManager<byte, MemoryManager<byte>>(
a.Data.Buffers[2].Memory, out var manager);
Console.WriteLine($"value buffer is owned by: {manager?.GetType().Name
?? "(managed array)"}");
return new WeakReference(manager);
}
// Stands in for the allocating work a consumer does while holding the spans.
static void DoWorkWhileHoldingSpans()
{
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
}
[MethodImpl(MethodImplOptions.NoInlining)]
static bool WithoutKeepAlive()
{
var array = BuildArray(100_000);
var owner = OwnerOf(array);
ReadOnlySpan<int> offsets = MemoryMarshal.Cast<byte,
int>(array.Data.Buffers[1].Span);
ReadOnlySpan<byte> values = array.Data.Buffers[2].Span;
DoWorkWhileHoldingSpans();
long total = 0;
for (int i = 0; i < 100_000; i++) total += offsets[i + 1] - offsets[i];
_ = values.Length;
return owner.IsAlive;
}
[MethodImpl(MethodImplOptions.NoInlining)]
static bool WithKeepAlive()
{
var array = BuildArray(100_000);
var owner = OwnerOf(array);
ReadOnlySpan<int> offsets = MemoryMarshal.Cast<byte,
int>(array.Data.Buffers[1].Span);
ReadOnlySpan<byte> values = array.Data.Buffers[2].Span;
DoWorkWhileHoldingSpans();
long total = 0;
for (int i = 0; i < 100_000; i++) total += offsets[i + 1] - offsets[i];
_ = values.Length;
bool alive = owner.IsAlive;
GC.KeepAlive(array);
return alive;
}
Console.WriteLine($"buffer owner still alive, without GC.KeepAlive:
{WithoutKeepAlive()}");
Console.WriteLine($"buffer owner still alive, with GC.KeepAlive:
{WithKeepAlive()}");
```
Run with tiered compilation disabled, since tier-0 keeps locals alive to the
end of the method and masks the problem:
```
DOTNET_TieredCompilation=0 dotnet run -c Release
```
Output:
```
value buffer is owned by: NativeMemoryManager
buffer owner still alive, without GC.KeepAlive: False
value buffer is owned by: NativeMemoryManager
buffer owner still alive, with GC.KeepAlive: True
```
`False` means the `NativeMemoryManager` was collected and finalized — i.e.
`Marshal.FreeHGlobal` ran on the buffer — while the two spans above were still
being read.
### Why it happens
Three behaviours combine:
1. `MemoryAllocator.Default` is a `NativeMemoryAllocator`, so any buffer
produced by an Arrow builder lives in `Marshal.AllocHGlobal` memory.
2. `NativeMemoryManager` frees that memory from a **finalizer**.
3. `ArrowBuffer.Span` (and the `Values` properties on the typed arrays) hand
out a raw span, which is a bare pointer into that native memory and keeps
nothing alive.
A span over a *managed* `byte[]` is safe here, because its interior pointer
is reported to the GC and roots the array. A span over native memory has no
such protection, so the only thing keeping the allocation alive is a reference
to the owning object — and in the code shape above there isn't one past the
point where the spans are taken.
Notably, `NativeMemoryManager` already suppresses the analyzer that exists
specifically to flag this:
```csharp
#pragma warning disable CA2015 // TODO: is this correct?
~NativeMemoryManager()
{
Dispose(false);
}
#pragma warning restore CA2015
```
[CA2015](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2015)
is *"Do not define finalizers for types derived from `MemoryManager<T>`"*, and
its rationale is exactly this failure mode.
### Why it is easy to miss
The freed pages usually remain mapped, so the span keeps returning
correct-looking bytes and nothing is observed. It faults only when the page is
genuinely returned to the OS.
We hit it as an `AccessViolationException` that killed a test host mid-run,
in a dictionary encoder that took two spans off a caller-supplied array and
then looped over a million rows while allocating. It appeared on a commit that
changed only Markdown, having passed on the three commits before it — the code
involved had been in place and "working" for some time.
### Note on `main`
The reference-counting layer added in #297 (`SharedMemoryHandle`,
`ArrowBuffer.Retain()`, `ArrowBuffer : IDisposable`) makes sharing explicit,
but does not close this: `SharedMemoryHandle` also defines a finalizer, and
`ArrowBuffer.Span` still returns a span that roots nothing. So the hazard
appears to remain on `main`.
### Possible directions
Any one of these would remove the failure mode; they are not mutually
exclusive:
- **Drop the finalizers** on `NativeMemoryManager` (and
`SharedMemoryHandle`), so that a missing release becomes a *leak* rather than a
free-under-live-span. This is what CA2015 prescribes, and it converts a
silent-corruption bug class into a diagnosable one. It is a behaviour change
for callers who never dispose, but the `IDisposable`/ref-counting work already
moves in that direction.
- **Make the lifetime-safe accessor the ergonomic one** — e.g. favour
`Memory<byte>` (which does root the manager) over `Span`, or provide a scope
type that holds the owner for the duration of a read.
- **Consider a managed-array allocator as the default**, reserving native
allocation for interop and explicitly-aligned paths. This has real costs for
alignment and the C Data Interface, so it is the least appealing of the three,
but it would make the question moot for consumers that never touch native
interop.
### Workaround for consumers
Keep the array reachable until after the last span read:
```csharp
var data = array.Data;
ReadOnlySpan<byte> values = data.Buffers[2].Span;
// ... work ...
GC.KeepAlive(array);
```
It is worth noting that this is an unwritten contract every consumer has to
rediscover independently, and that code violating it looks correct and passes
tests.
### Environment
- Apache.Arrow 23.0.0 (NuGet)
- .NET 8 target, .NET 10 SDK
- macOS 15 / arm64
- Also expected on `main` by inspection (see the note above)
--
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]
