GDAL v3.13.1 + thread-safe GdalBase - #232
Merged
Merged
Conversation
MaxRev-Dev
marked this pull request as ready for review
June 8, 2026 21:29
MaxRev-Dev
force-pushed
the
release/v3.13.1
branch
from
June 8, 2026 21:32
9e36821 to
1cd9569
Compare
Apply double-checked locking to ConfigureGdalDrivers and ConfigureAll so parallel callers no longer race into native AllRegister/RegisterAll. The public IsConfigured property is preserved; the backing flag is now a volatile bool guarded by a private lock object. Add a concurrent-callers test to BinaryLoadingTests that fans 16 threads through a Barrier and asserts the registered driver count remains stable.
MaxRev-Dev
force-pushed
the
release/v3.13.1
branch
from
June 8, 2026 21:33
1cd9569 to
7d71c13
Compare
There was a problem hiding this comment.
Pull request overview
Updates the build configuration to GDAL v3.13.1 and refactors GdalBase initialization to reduce races when multiple callers initialize GDAL concurrently, adding an XUnit test to exercise concurrent ConfigureAll() calls.
Changes:
- Bump GDAL version to 3.13.1 in shared build options.
- Add locking +
volatileflag to makeGdalBase.ConfigureGdalDrivers()/ConfigureAll()safe under concurrent access. - Add
ConfigureAll_IsThreadSafetest that runs 16 concurrent callers gated by aBarrier.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
shared/GdalCore.opt |
Updates GDAL_VERSION to 3.13.1 for the native/runtime build pipeline. |
compile/GdalConfigureAll.cs |
Adds locking/volatile state to avoid concurrent native driver registration. |
tests/MaxRev.Gdal.Core.Tests.XUnit/BinaryLoadingTests.cs |
Adds a concurrency test to validate ConfigureAll() under contention. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address PR #232 review feedback: - ConfigureAll() now gates its fast-path on a dedicated _isFullyConfigured flag instead of _isConfigured. _isConfigured is set inside ConfigureGdalDrivers() before Proj.Configure() runs, so concurrent callers could return on the fast-path while the first thread was still configuring PROJ inside the lock - using PROJ before search paths were set. _isFullyConfigured is set only after Proj.Configure() completes. - ConfigureAll_IsThreadSafe test: barrier.SignalAndWait() now uses a 30s timeout and throws TimeoutException instead of hanging indefinitely if a task dies before reaching the barrier, making failures deterministic.
Async counterpart of GdalCli.Run. Spawns the tool, reads stdout/stderr concurrently (avoiding the latent pipe-buffer deadlock in the sync path), awaits exit, and reports output via the existing Action<string> callbacks. - Supports CancellationToken: on cancellation the tool is terminated (TryKill) and OperationCanceledException is surfaced after draining the pending reads. - Cross-framework exit wait: uses Process.WaitForExitAsync on net5.0+, and a TaskCompletionSource + Process.Exited fallback on netstandard2.0/2.1 (the shipped CLI target) where WaitForExitAsync is unavailable. - Tests.CLI smoke test now also exercises the async path.
No win-arm64 native runtime is shipped; Windows on ARM64 runs the win-x64 build via emulation. This makes PathInitializer.GetRuntimeNativeDirectory consistent with GdalCli.GetRuntimeRid, which already returns win-x64 for all Windows architectures.
The previous comment claimed Windows ARM64 runs the win-x64 build 'via emulation'. That is misleading: emulation is a per-process decision, and x64 native libraries load only into an x64 process (including an x64-emulated process on Windows ARM64), never into a natively-running ARM64 process. Behavior is unchanged - win-x64 is still the only Windows native runtime shipped; only the explanation is corrected.
Address PR #232 review: the synchronous Run() read stdout to completion before stderr, which can deadlock if the tool fills its stderr pipe buffer while Run() is blocked reading stdout. Now starts both ReadToEndAsync reads before WaitForExit (mirrors RunAsync), so neither pipe can block the tool.
This was referenced Jun 23, 2026
This was referenced Jul 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
shared/GdalCore.opt).GdalBaseinitialization to use double-checked locking so concurrent callers no longer race into the nativeGdal.AllRegister/Ogr.RegisterAllregistration path.BinaryLoadingTests.ConfigureAll_IsThreadSafecovering 16 concurrent callers gated by aBarrier.Implementation notes
IsConfiguredis now an expression-bodied getter over avolatile boolbacking field; lock is a privateobject.ConfigureGdalDriversandConfigureAllboth follow the DCL pattern (fast-path read -> lock -> re-check). C#lockis reentrant soConfigureAll->ConfigureGdalDriversdoes not deadlock.ConfigureGdalDatais unchanged - it just forwards toGdal.SetConfigOptionwhich is already idempotent.Test plan
dotnet build package-build/gdalcore.loader.final.csproj -c Release-> 0 warnings, 0 errors across netstandard2.0/2.1, net461, net6/7/8/9/10dotnet build tests/MaxRev.Gdal.Core.Tests.XUnit/MaxRev.Gdal.Core.Tests.XUnit.csproj -c Release-> 0 errors (only pre-existing NU1603 nuget resolution warnings)