Skip to content

ArrowBuffer spans do not keep native memory alive: NativeMemoryManager's finalizer can free a buffer while a Span over it is in use #397

Description

@CurtHagenlocher

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 NativeMemoryManagerwhose 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:

<ProjectSdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net8.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
</PropertyGroup>
<ItemGroup>
<PackageReferenceInclude="Apache.Arrow"Version="23.0.0" />
</ItemGroup>
</Project>

Program.cs:

usingSystem.Buffers;usingSystem.Runtime.CompilerServices;usingSystem.Runtime.InteropServices;usingApache.Arrow;staticStringArrayBuildArray(introws){varb=newStringArray.Builder();for(inti=0;i<rows;i++)b.Append($"row-{i}");returnb.Build();}// WeakReference to the MemoryManager that owns the array's value buffer.staticWeakReferenceOwnerOf(StringArraya){MemoryMarshal.TryGetMemoryManager<byte,MemoryManager<byte>>(a.Data.Buffers[2].Memory,outvarmanager);Console.WriteLine($"value buffer is owned by: {manager?.GetType().Name??"(managed array)"}");returnnewWeakReference(manager);}// Stands in for the allocating work a consumer does while holding the spans.staticvoidDoWorkWhileHoldingSpans(){GC.Collect();GC.WaitForPendingFinalizers();GC.Collect();}[MethodImpl(MethodImplOptions.NoInlining)]staticboolWithoutKeepAlive(){vararray=BuildArray(100_000);varowner=OwnerOf(array);ReadOnlySpan<int>offsets=MemoryMarshal.Cast<byte,int>(array.Data.Buffers[1].Span);ReadOnlySpan<byte>values=array.Data.Buffers[2].Span;DoWorkWhileHoldingSpans();longtotal=0;for(inti=0;i<100_000;i++)total+=offsets[i+1]-offsets[i];_=values.Length;returnowner.IsAlive;}[MethodImpl(MethodImplOptions.NoInlining)]staticboolWithKeepAlive(){vararray=BuildArray(100_000);varowner=OwnerOf(array);ReadOnlySpan<int>offsets=MemoryMarshal.Cast<byte,int>(array.Data.Buffers[1].Span);ReadOnlySpan<byte>values=array.Data.Buffers[2].Span;DoWorkWhileHoldingSpans();longtotal=0;for(inti=0;i<100_000;i++)total+=offsets[i+1]-offsets[i];_=values.Length;boolalive=owner.IsAlive;GC.KeepAlive(array);returnalive;}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 managedbyte[] 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:

#pragma warning disable CA2015// TODO: is this correct?~NativeMemoryManager(){Dispose(false);}
#pragma warning restore CA2015

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:

vardata=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)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions