Skip to content

fix(csharp): prevent freeing driver library twice - #4976

Merged
CurtHagenlocher merged 2 commits into
apache:mainfrom
amoeba:csharp-importer-double-free
Oct 28, 2026
Merged

CurtHagenlocher merged 2 commits into
apache:mainfrom
amoeba:csharp-importer-double-free

Conversation

@amoeba

@amoeba amoeba commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

When CAdbcDriverImporter.Load is called with canUnload: true and the library doesn't export the requested entrypoint, the netstandard2.0 build (used on .NET Framework) frees the library twice: once before throwing and again in the finally block. That build uses its own NativeLibrary shim, whose GetExport returns IntPtr.Zero for a missing export instead of throwing, so it reaches a branch the net8.0/net10.0 builds never do. Freeing twice can unload a library that something else in the process still uses.

This PR adds a test for this case and leaves freeing the library to the finally block.

Bug found and PR created with Opus 5.5.

@amoeba

amoeba commented Oct 28, 2026 •

Copy link
Copy Markdown
Member Author

CI should fail on Windows on a9edd1e.

@amoeba

amoeba commented Oct 28, 2026 •

Copy link
Copy Markdown
Member Author

From https://github.com/apache/arrow-adbc/actions/runs/40781480559/job/123143975815?pr=4976

[xUnit.net 00:00:02.59]     Apache.Arrow.Adbc.Tests.CAdbcDriverImporterUnloadTests.Load_CanUnload_MissingEntrypoint_KeepsOtherReferences [FAIL]
  Failed Apache.Arrow.Adbc.Tests.CAdbcDriverImporterUnloadTests.Load_CanUnload_MissingEntrypoint_KeepsOtherReferences [227 ms]
  Error Message:
   The library was unloaded while another reference to it was still held
  Stack Trace:
     at Apache.Arrow.Adbc.Tests.CAdbcDriverImporterUnloadTests.Load_CanUnload_MissingEntrypoint_KeepsOtherReferences() in D:\a\arrow-adbc\arrow-adbc\csharp\test\Apache.Arrow.Adbc.Tests\CAdbcDriverImporterUnloadTests.cs:line 60

The error isn't a direct check but hopefully the way the test is structured makes this make sense.

@amoeba
amoeba marked this pull request as ready for review October 28, 2026 22:54

@CurtHagenlocher CurtHagenlocher left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@CurtHagenlocher
CurtHagenlocher merged commit 57cb036 into apache:main Oct 28, 2026
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants