Skip to content

feat(dotnet): make the .NET binding work on macOS and Linux via the pure-C shimtion#253

Open
zzcgumn wants to merge 21 commits into
developfrom
feat/dot_net_integration
Open

feat(dotnet): make the .NET binding work on macOS and Linux via the pure-C shimtion#253
zzcgumn wants to merge 21 commits into
developfrom
feat/dot_net_integration

Conversation

@zzcgumn

@zzcgumn zzcgumn commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

What and why

DDS_Core is a complete, idiomatic .NET wrapper — 46 P/Invokes, a full blittable
DataModel, SafeHandle lifetimes. But it only ever worked on Windows, because the
native library it loaded (dds_native.dll) is built solely by
solution/dds_native.vcxproj under MSVC.

//jni:dds_shared already builds a self-contained shared library on all three
platforms. This PR closes the ABI gap between the two so the existing managed API
runs unchanged on macOS, Linux, and Windows.

The gap

The binding's fourteen modern entry points bound dds_* from dds_api.hpp. Those
symbols are not exported on Linux or macOS — the export lists pin the ABI to
dll.h + dds_c_api.h:

$ nm -gU bazel-bin/jni/libdds.dylib | grep dds_
_dds_c_calc_dd_table  _dds_c_calc_par  _dds_c_create_solvercontext_default
_dds_c_destroy_solvercontext  _dds_c_solve_board

So all fourteen would have thrown EntryPointNotFoundException off Windows. The
other 30 P/Invokes are the legacy flat API and already resolved.

Two causes: five functions existed only under dds_c_* names, and nine had no shim
equivalent at all. This was invisible because Windows has no .lds branch — its DLL
exports the broader DLLEXPORT set, including dds_api.hpp.

What changed

Native (library/src/api/dds_c_api.{h,cpp}) — nine functions added to the shim:
config-based context creation, calc_dd_table_pbn, TT configure/resize/clear, both
resets, and the two logging passthroughs. Each follows the file's existing pattern —
null guards plus a catch-all so no C++ exception unwinds through the C ABI.
SolverConfig is decomposed into scalars rather than mirrored as a C struct, keeping
the shim pointer-only and POD-only; TTKind crosses as an int.

Export lists regenerate from the headers, so //jni/tests:export_set_test validated
the result with no wiring change.

Managed (dotnet/DDS_Core) — the fourteen P/Invokes retarget onto dds_c_* via
EntryPoint, so no call site in DDS.cs or SolverContext.cs moves. The library
name becomes dds (letting .NET's probing supply the lib prefix and per-OS
extension). dds_log_append marshals UTF-8 explicitly instead of relying on the
platform-dependent CharSet.Ansi default. A resolver honours DDS_LIBRARY_PATH
the counterpart of the JVM binding's -Ddds.library.path — falling back to default
probing when unset, and failing loudly with the attempted path when set but wrong.

Also removes two commented-out calc_par declarations: they are plain C++ with no
extern "C" or DLLEXPORT, so they were never bindable on any platform.

Two pre-existing memory bugs found and fixed

Adding coverage for the shim surfaced two faults in transposition-table lifecycle.
Both predate this PR, both are reachable from the existing public API, and neither
involves the new shim entry points
— each was reproduced through the
reference-taking C++ API with no dds_c_* call in the trace.

1. Use-after-free in clear_tt() — affects the default configuration

allocated: TransTable{L,S}::make_tt
freed:     TransTable{L,S}::return_all_memory   ← clear_tt()
read:      TransTable{L,S}::lookup              ← next solve

clear_tt() returned the TT's memory but kept the instance.
SearchContext::trans_table() only checks whether tt_ is non-null, so it handed
that husk straight back and the next lookup read freed pools.

This hits the default Large TT — no TT-kind switch required. The sequence is just
clear_tt() followed by a solve, which the .NET binding exposes directly as
SolverContext.ClearTT() then SolveBoard(). Caught by ASan on Linux CI.

The fix disposes the instance, making the already-documented "recreates lazily on
demand" behaviour real: tt_ becomes null and the next trans_table() rebuilds from
the owner's config. Nothing is lost — kind and memory limits live in
SolverContext::cfg_, not the instance, and the destructor returns the memory either
way. Disposal also keeps the fix off the hot path, since ab_search calls
trans_table() per lookup.

2. Null dereference in TransTableS::reset_memory() — Small TT only

solve → configure_tt(Small) → clear_tt() → reset_for_solve()
    → null deref in TransTableS::init_tt()

reset_memory() ignored tt_in_use_ and re-initialised over freed pools. Fixed with
the guard TransTableL::reset_memory() already had (pool_ == nullptr), which is why
only the Small TT was affected.

Reviewer note: fix (1) largely subsumes this one — with the instance disposed,
maybe_trans_table() returns null and reset_for_solve() no longer reaches a
memory-less table. The guard is retained as defence in depth and for symmetry with
TransTableL, but it is no longer load-bearing. Reasonable to ask for it to be
dropped as unreachable.

Testing

  • //library/tests:dds_c_api_test (new, 13 tests) — null-handle and null-argument
    rejection, both TT kinds, reconfiguration, resets, logging, PBN/binary agreement,
    plus regressions for both bugs above.
  • dotnet/DDS_Core.Tests (new, 19 tests) — layout, smoke, and lifecycle. Between
    them every one of the fourteen retargeted P/Invokes is exercised, so a missing
    EntryPoint fails here rather than in a consumer.
  • CI — managed build and test added to Linux, Windows, and macOS. Windows matters
    most: it is the platform this change could regress, and the Bazel-built dds.dll
    path is covered by nothing else (the JNI tests are target_compatible_with-excluded
    there). macOS is included because it is the only CI coverage of AArch64.

The layout tests carry the most weight. The managed structs had only ever been
exercised against the MSVC ABI, and a mismatch on SysV or AArch64 corrupts results
silently rather than throwing. Expected sizes and offsets are derived from the C
headers via offsetof/sizeof, not from running the C# — asserting what the code
already does would prove nothing. They pass, which is the first confirmation the
layouts are correct off Windows.

Verified locally on macOS/arm64: ASan and TSan clean across //library/tests/...,
59/59 Bazel tests, 19/19 .NET tests.

Notes for reviewers

  • A claim in the commit history is corrected. <Platforms>x64</Platforms> was
    not blocking Apple Silicon, as I initially assumed. It only declares the IDE's
    valid platform list; it never set <PlatformTarget>, so the assembly was already
    AnyCPU and arm64 builds already worked. That change is declarative, not unblocking.
  • The test project targets net8.0 with <RollForward>Major</RollForward> so it
    also runs where only a newer major runtime is installed. CI pins an 8.0 SDK, where
    the property is inert.
  • The resolver registers from a static constructor, not [ModuleInitializer]: the
    runtime guarantees a type initializer runs before that type's first P/Invoke, which
    is the same guarantee without CA2255's objection to module initializers in libraries.
  • CI resolves the native library via bazel info bazel-bin rather than the
    bazel-bin symlink, which is configuration-dependent and moves between --configs.
  • Local ASan on macOS needs DEVELOPER_DIR exported
    (/Applications/Xcode.app/Contents/Developer), otherwise Bazel's Xcode locator
    fails against Xcode 26.5 and every sanitizer target fails to build. Bug (1) reached
    CI because of this. Worth adding to the dev setup notes.

Deliberately out of scope

  • NuGet packaging. RID-specific assets (runtimes/<rid>/native/…) and a
    multi-platform package are a follow-up, mirroring how the jar followed the JVM
    shared library.
  • Retiring solution/dds_native.vcxproj. Still present for interactive VS
    debugging; DDS_Core.slnx still build-depends on it, and it now emits a library
    name nothing loads. (That solution also does not build under dotnet build because
    it includes a C++ project needing VS MSBuild — pre-existing, confirmed against the
    base.)
  • Moving dds_shared out of //jni. It now serves three ecosystems, but renaming
    is best done with the packaging work rather than mid-ABI-change.

Specs

specs/dds-public-api.md had three claims this work falsifies (the shim as "a thin
subset… no *PBN twins and no TT configure"; .NET binding dds_*; the PBN pairing not
extending to the shim) — all corrected. Adds specs/dotnet-binding.md, which the repo
lacked despite having one for the Python and JVM bindings.

Closes #227

@zzcgumn
zzcgumn requested a review from tameware July 20, 2026 11:49
@zzcgumn zzcgumn self-assigned this Jul 20, 2026
@zzcgumn

zzcgumn commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

@timanderson, your feedback on this would be much appreciated.

@zzcgumn

zzcgumn commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

@mortensp, I don't think this breaks the visual studio build. Any chance you can check?

@timanderson

Copy link
Copy Markdown

Thanks for doing this @zzcgumn , I checked out the PR and it works on macOS. I have no idea what a "pure-C shimtion" is though!

I am not sure that the changes the AI made to DdsNative.cs were necessary, despite the claim that "So all fourteen would have thrown EntryPointNotFoundException off Windows" this was not the case with the tests I did before on the Mac as you can see from the output that I pasted, unless there is something I am missing (quite possible).

Rather than manually copying the shared library file or setting DDS_LIBRARY_PATH I personally prefer to modify the .csproj to copy the shared library to the output, which you can do with conditions so the right library is copied for the OS on which you are running. Not a big deal.

As the AI notes, if you run dotnet build in the DDS_Core directory it will fail because it will look at the .slnx file which does a Windows-specific build. It is easy to get round this by typing:

dotnet build DDS_Core.csproj

but adds a bit of friction. But some on Windows may prefer to load the solution and not use Bazel which I understand. It would make sense to make the DLL name and the output location the same whether using Bazel or MSVC to build it.

As it stands I think the PR could end up being a little bit annoying for both the Windows and non-Windows users.

Incidentally, .NET 8 goes out of support in November despite being an LTS release (blame Microsoft).

@zzcgumn

zzcgumn commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

As it stands I think the PR could end up being a little bit annoying for both the Windows and non-Windows users.

I am only using the C++ and Python interfaces actively in my own work. What you say about build artefact locations makes sense, but I am not in a position to have a good judgement on how to improve. I am thinking that the best way forward is to merge this and hope that people who use .net actively can make contributions that reduce the friction. Does this make sense?

@timanderson

Copy link
Copy Markdown

I think the idea for Windows users is that they can open DDS_Core.slnx in Visual Studio, build, and everything works. The PR does break this. Actually it was already slightly broken for me; I was getting linker errors until I added deal_fanout.cpp and deal_fanout.hpp to the solution; I am not sure why they were missing. But after doing that it works. If I try that having checked out the PR I get a warning about "unknown project configuration mappings" and Visual Studio no longer builds the .NET projects; this is because of changes made to the project configuration. I think this should be fixed before the PR is merged.

zzcgumn and others added 20 commits July 21, 2026 13:50
The .NET binding calls fourteen modern context functions, but the shim
covered only five, so the other nine had no C-ABI equivalent and the
Unix shared library could not satisfy them.

Adds dds_c_create_solvercontext, dds_c_calc_dd_table_pbn, the TT
configure/resize/clear trio, both resets, and the two logging
passthroughs. Each follows the file's existing pattern: null guards
before use and a catch-all so no C++ exception unwinds through the C
ABI boundary.

SolverConfig is decomposed into three int parameters rather than
mirrored as a C struct — passing a struct by value is the ABI question
this shim exists to avoid, and a mirror type would be a second
definition to keep in sync. TTKind crosses as an int; the C++ enum
class and the C# enum already agree on 0 = Small, 1 = Large.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Publishes the nine shim symbols added in the previous commit through
the Unix export lists, so libdds.{so,dylib} exports all fourteen
dds_c_* entry points instead of five.

Both .lds files are generated artifacts, regenerated with
gen_export_lists.py rather than hand-edited. export_set_test derives
its expectations from the headers, so it validated the result with no
wiring change; the FFM smoke tests confirm the JVM binding is
unaffected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The shim had no dedicated C++ test — its five original functions were
covered only indirectly via the JVM smoke tests. That was tolerable for
five thin forwarders, less so for fourteen, and the new group includes
the shim's first void-returning and first string-taking entry points
whose null guards nothing exercised.

Covers null-handle and null-argument rejection, both TT kinds, TT
reconfiguration, the resets, logging, and PBN/binary DD-table
agreement. The reference board matches DdsSmokeTest.java so the JVM,
.NET, and C++ bindings share one fixture.

Depends on //library/src:dds explicitly rather than only transitively
through dds_c_api: dds.cpp carries the constructor that initializes
static solver memory, and without a direct edge the linker drops it.

Includes one DISABLED_ test recording a pre-existing library bug found
while writing this: on a Small TT, clear_tt() releases the pools and a
following reset_for_solve() re-inits over them, dereferencing null in
TransTableS::init_tt(). Verified reachable through the C++ API with no
dds_c_* call involved, so it predates this shim and affects the
existing Windows .NET path. Disabled rather than deleted so the fault
stays recorded without breaking the build.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
return_all_memory() frees pw_/pn_/pl_ and clears tt_in_use_, leaving
make_tt() to reallocate lazily before the next lookup. reset_memory()
did not honour that flag: it called init_tt() unconditionally, which
reads pw_[0] and segfaults on the freed pools.

Reachable from the public API as
  configure_tt(Small) -> clear_tt() -> reset_for_solve()
with no shim involved, so it also affects the existing C++ and Windows
.NET paths. The Large TT was never affected because
TransTableL::reset_memory() already guards the equivalent case with
`pool_ == nullptr`; this adds the direct analogue for the Small TT.

Found while adding shim coverage; the previously DISABLED_ regression
test in dds_c_api_test is now enabled and passing.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The fourteen modern P/Invokes bound dds_* from dds_api.hpp, which the
shared library does not export on Linux or macOS — every one of them
would have thrown EntryPointNotFoundException there. They now bind the
dds_c_* shim via EntryPoint, so the same managed code works on all
three platforms.

Managed method names are unchanged, so no call site in DDS.cs moves.
The one exception is context creation, which takes scalars now that
SolverConfig no longer crosses the ABI; SolverContext's constructor
unpacks the config it already holds. TTKind needs no change: it is
`enum TTKind : int` and marshals as the shim's int parameter.

Also renames the library to "dds" (letting .NET's probing supply the
lib prefix and per-OS extension), makes dds_log_append's UTF-8
marshalling explicit instead of relying on the platform-dependent
CharSet.Ansi default, and deletes the commented-out calc_par and
calc_par_from_table declarations — those are plain C++ with no
extern "C" or DLLEXPORT, so they were never bindable on any platform.

The 30 legacy flat-API P/Invokes are untouched; they already resolve.

Note: DDS_Core.slnx still build-depends on solution/dds_native.vcxproj,
which now emits a library name nothing loads. Retiring that project is
deferred per the plan.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Default probing stays in force — that is what a NuGet package laying
the library out under runtimes/<rid>/native will rely on — but setting
DDS_LIBRARY_PATH to a full path now overrides it, so tests and
development builds can bind against a freshly-built
bazel-bin/jni/libdds.dylib without installing anything. This is the
.NET counterpart of the JVM binding's -Ddds.library.path.

If the variable is set but the library will not load, the failure is
raised naming the attempted path rather than falling through to
probing: a typo in a test script should not surface later as a missing
entry point.

Registered from DdsNative's static constructor rather than a
[ModuleInitializer]: the runtime guarantees a type initializer runs
before that type's first P/Invoke, so it is equally safe while being
lazy rather than eager — and CA2255 warns against module initializers
in library code. The task sketched the module-initializer form; this
is the same guarantee without the warning.

Verified end to end on macOS/arm64: solving the reference board through
SolverContext against the Bazel-built library returns 13 tricks, an
unset variable falls back to probing, and a wrong one fails loudly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both projects declared <Platforms>x64</Platforms> while emitting an
architecture-neutral assembly, so the declaration did not match what
was built.

Correcting the task's premise: this pin was NOT blocking Apple
Silicon. <Platforms> only declares the valid platform list for the
IDE/solution; it never set <PlatformTarget>, so the assembly was
already AnyCPU (PE machine 0x014c) and both `dotnet build` and
`-p:Platform=AnyCPU` already succeeded on arm64. The change is
therefore declarative — it stops the project claiming an architecture
it does not target, and unrestricts the IDE configuration list — not
an unblocking fix.

Output paths verified unchanged: Directory.Build.props interpolates
$(platform) into BaseIntermediateOutputPath, but Directory.Build.props
is imported before the SDK defines Platform, so that property was
already empty and the segment already collapsed. No empty or doubled
path segments after the change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Nineteen tests covering the retargeted binding, all passing on
macOS/arm64 against the Bazel-built libdds.dylib.

The layout tests carry the most weight. The managed structs had only
ever been exercised against the MSVC ABI, and a mismatch on SysV or
AArch64 corrupts results silently rather than throwing — so the smoke
tests alone could not catch it. Expected sizes and offsets are derived
from the C headers via offsetof/sizeof, not from running the C#, since
asserting what the managed code already does would prove nothing. They
pass, which is the first confirmation the layouts are correct off
Windows.

Between the smoke and lifecycle tests every one of the fourteen
retargeted P/Invokes is exercised, so a missing EntryPoint fails here
rather than in a consumer. Includes a managed regression for the
Small-TT ClearTT/ResetForSolve crash fixed earlier in this branch, and
a one-context-per-thread concurrency check.

Targets net8.0 to match the library, with <RollForward>Major</RollForward>
so the suite also runs where only a newer major runtime is installed
(the Homebrew SDK case). This replaces the multi-targeting the task
suggested: multi-targeting would have run both frameworks and still
needed the roll-forward for the net8.0 pass, so it removed nothing.
Where an 8.0 runtime exists, as on CI, the property has no effect.

Output paths are inherited from Directory.Build.props rather than
overridden: Build/int is gitignored where the SDK-default obj/ is not,
and overriding BaseIntermediateOutputPath in the csproj comes too late
for MSBuild (MSB3539).

Note: DDS_Core.slnx does not build under `dotnet build` because it
includes the C++ dds_native.vcxproj, which needs Visual Studio's
MSBuild. That predates this change; the projects build individually,
which is what CI and `dotnet test` use.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Windows is the reason this is not deferred to the packaging follow-up.
It was the only platform where the .NET binding worked, via
dds_native.dll and dds_api.hpp's dds_* symbols; after the shim
retarget it binds dds_c_* in the Bazel-built dds.dll, and no existing
test covers that path — the JNI smoke tests and export_set_test are
target_compatible_with-excluded on Windows.

macOS is included rather than skipped because it is the only CI
coverage of the AArch64 ABI, which is exactly what the managed
struct-layout tests guard: a mismatch there corrupts results silently
instead of throwing. With Linux (SysV x86-64) and Windows (Win64),
all three ABIs are now exercised.

Each job resolves the native library through `bazel info bazel-bin`
rather than the bazel-bin convenience symlink, which is
configuration-dependent and moves when a different --config is used,
and fails with a clear message if the library is absent rather than
letting dotnet report a confusing missing-entry-point error.

The SDK is pinned to 8.0.x to match what DDS_Core targets; the test
project's RollForward only takes effect where no 8.0 runtime exists,
which is not the case on these runners.

Verified locally on macOS/arm64 by running the workflow's exact
command sequence: 19/19 tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three claims in dds-public-api.md were falsified by this work and are
corrected: the shim is no longer "a thin subset ... no *PBN twins and
no TT configure"; .NET no longer P/Invokes dds_* from dds_api.hpp; and
the PBN pairing now does extend to the shim, via
dds_c_calc_dd_table_pbn. jni-ffm-binding.md notes that the Windows
DLLEXPORT superset is no longer relied on by any shipped binding, and
that //jni:dds_shared serves .NET as well despite its location.

Adds specs/dotnet-binding.md, a capability spec the repo lacked
despite having one for the Python and JVM bindings. It records which
ABI layer is bound and why, the single "dds" library name,
DDS_LIBRARY_PATH resolution, SafeHandle ownership and
one-context-per-thread, and the invariants the layout, smoke, and
lifecycle tests enforce. Capability-level only — no per-method
signatures, which stay in doxygen and docs/.

docs/dotnet_interface.md gains a build-and-load section covering the
Bazel target, per-OS artifacts, both load paths, and the advice to
resolve via `bazel info bazel-bin` rather than the
configuration-dependent symlink. It also states which native symbols
are used, since binding dds_api.hpp is what previously made the
wrapper Windows-only.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
clear_tt() returned the transposition table's memory but kept the
instance. SearchContext::trans_table() only checks whether tt_ is
non-null, so it handed that husk straight back and the next lookup
read freed pools. Caught by ASan on Linux CI:

  allocated: TransTable{L,S}::make_tt
  freed:     TransTable{L,S}::return_all_memory   <- clear_tt()
  read:      TransTable{L,S}::lookup              <- next solve

Disposing the instance instead makes the documented "recreates lazily
on demand" behaviour real: tt_ becomes null, so the next
trans_table() rebuilds from the owner's config. Nothing is lost, since
the kind and memory limits live in SolverContext::cfg_ rather than in
the TT instance, and the destructor returns the memory either way.
Disposal also keeps the fix off the hot path — ab_search calls
trans_table() per lookup, so re-checking allocation state there would
have cost more than it saved.

This is pre-existing and independent of the .NET work: reproduced
through the reference-taking C++ API as dds_clear_tt() followed by
dds_solve_board(), with no dds_c_* call involved. It affects the
default Large TT, so the existing .NET binding has the same latent
fault via SolverContext.ClearTT() then SolveBoard().

Note this supersedes the TransTableS::reset_memory guard added earlier
in this branch as the active fix for that path: with the instance
disposed, maybe_trans_table() returns null and reset_for_solve() no
longer reaches a memory-less table. That guard is left in place as
defence in depth, mirroring the one TransTableL::reset_memory already
had.

Verified: ASan and TSan clean across //library/tests/..., 59/59 Bazel
tests, 19/19 .NET tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
clear_tt() performed the disposal with a raw const_cast on search_,
bypassing SolverContext::dispose_trans_table() twelve lines above. That
skipped the "tt:dispose" log entry and the tt_disposes stats counter, so
the counter under-reported and the log trace no longer recorded that the
TT went away. configure_tt() already calls dispose_trans_table() for the
same operation; the two disposal paths are now consistent.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The heap-use-after-free fix changed clear_tt() from return_all_memory()
on the live TT to disposing the instance, but the spec still documented
the old behaviour in two places -- including an explicit "reuse after
clear_tt() is unsafe" warning and "No test guards this today", both of
which are now false. A caller reading the spec would work around a
hazard that no longer exists and treat a supported path as broken.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two tests claimed to guard TransTableS::reset_memory()'s tt_in_use_ early
return, but neither reached it: clear_tt() now disposes the TT, so
reset_for_solve() sees no table and never calls reset_memory(). Both
would have passed with the guard reverted -- verified by temporarily
removing it.

Add a direct unit test driving TransTableS through
make_tt/return_all_memory/reset_memory, which segfaults without the
guard (verified), and reword the two indirect tests plus the guard's
comment to describe what they actually cover.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The comment claimed the //library/src:dds edge retains an
__attribute__((constructor)) that initializes static solver memory. All
three parts were wrong: on Windows dds.cpp takes the DllMain branch, and
DllMain never runs for a statically-linked cc_test; a plain deps edge
does not force retention of an unreferenced object file (that needs
alwayslink); and dds_c_api already depends on //library/src:dds, so the
edge changes nothing. The cited failure mode belongs to the TransTableS
reset guard, which is unrelated to static-memory init.

The test passes because SolverContext(SolverConfig) allocates its own
ThreadData. Say that instead.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ThrowIfError carried [Conditional("DEBUG")], which elides every call
site in a Release build. In the configuration consumers actually ship,
SolveBoard/CalcDdTable/CalcPar returned the raw RETURN_* code and never
threw -- the opposite of what specs/dotnet-binding.md promises.

Drop the attribute so the documented contract holds in every build, and
add a test driving RETURN_SOLNS_WRONG_HI that fails in Release if the
check is ever made conditional again. The spec bullet now says so
explicitly.

Note this is consumer-visible: code that previously inspected return
codes in Release will now see exceptions on failure paths.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The `?? throw` on both constructors was dead code. A P/Invoke returning
a SafeHandle-derived type never yields null -- the marshaller constructs
an instance and stores whatever pointer came back -- so when
dds_c_create_solvercontext returns NULL the caller got a handle with
IsInvalid == true, the throw never fired, and every subsequent call
passed IntPtr.Zero into the shim to be null-guarded into
RETURN_UNKNOWN_FAULT.

Test the handle instead, disposing the failed one, and assert on the
successful path that construction yields a live pointer.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The lookup added to ResetAfterReturnAllMemoryIsInert used
CreateTestAggrTarget, which fills aggr_target with {0x1111, 0x2222,
0x3333, 0x4444}. lookup() indexes aggp_[aggr_target[ss]], and aggp_ has
8192 (2^13) slots because aggr_target is a 13-bit rank mask -- 0x2222 =
8738 reads past the end. With an all-zero hand_dist the lookup key
matches the rebuilt tree root, so it reached that indexing and crashed
under a page layout where the overrun hits unmapped memory (it read
adjacent heap and passed in fastbuild).

Use an in-range all-zero aggr_target. Confirmed under ASan that the test
is clean with the reset guard and still SEGVs without it, so it remains
a genuine guard for the fix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ThrowIfError is deliberately not [Conditional("DEBUG")], so a regression
to the conditional form only changes behaviour in Release. CI built
DDS_Core in Debug only, where DEBUG is defined and the call is emitted
regardless -- so SolveBoard_WithInvalidSolutions_Throws, whose whole
purpose is to guard that Release still throws, would still pass against a
reverted [Conditional("DEBUG")]. The guarantee was untested.

Add a second `dotnet test -c Release` run on all three platforms so the
regression actually fails CI, and make the test's "every build
configuration" claim literally true.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The reset-semantics note still said clear_tt() "returns all TT memory to
the system", while the function's @brief body and the .cpp now describe
disposing the TT instance. Reword the note to match so the header does
not describe two different behaviours for the same call.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@zzcgumn
zzcgumn force-pushed the feat/dot_net_integration branch from 56662e8 to a18255a Compare July 21, 2026 12:50
@zzcgumn

zzcgumn commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

I don't have access to Visual Studio, only Visual Studio Code which as far as I can tell does not understand the DDS_Core.slnx build file. @timanderson or @mortensp , is anyone of you able to fix the visual studio solution files? I am happy for you to push to this branch or open a pull request into it.

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.

Make dotnet DDS_Core cross-platform

2 participants