ArrowBuffer spans do not keep native memory alive: NativeMemoryManager's finalizer can free a buffer while a Span over it is in use
- Dominant language
- C#
- Stars
- 39
- Forks
- 30
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 16
Description
### Summary
A `ReadOnlySpan` 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
Exe
net8.0
enable
```
`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>(
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 offsets = MemoryMarshal.Cast(array.Data.Buffers[1].Span);
ReadOnlySpan 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 offsets = MemoryMarshal.Cast(array.Data.Buffers[1].Span);
ReadOnlySpan 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`"*, 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` (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 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)
Contributor guide
Research direction
Start with NativeMemoryManager, SharedMemoryHandle, MemoryAllocator.Default, and ArrowBuffer.Span, then run the supplied repro with DOTNET_TieredCompilation=0. Compare the finalizer and ownership paths described in the issue and determine which lifetime policy should be adopted; done means spans cannot outlive the native allocation and the reproduction no longer permits a freed owner during reads.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend, performance
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100