libslvs C API aborts foreign host apps instead of returning an error #1766

Summary

slvs is a library. It runs inside applications that SolveSpace does not
write, ship, or control. When it is handed input it will not accept, it
calls Platform::FatalError() (and therefore abort()) and terminates
that host application. Counting the sources this library actually
compiles, there are 108 such exit points: 55 direct
Platform::FatalError calls in src/slvs/lib.cpp, plus 53 ssassert
sites in the kernel that reach it through AssertFailure. Exactly one
abort() exists in the whole set, inside FatalError itself, so every
one of them converges on a single function.

For the SolveSpace application this is a deliberate and reasonable choice,
and I am not asking you to change it there. For a library that can be used
with foreign apps, it is a different matter: the host process dies, takes
the user's unsaved work with it, and the calling code has no way to
prevent, catch, or recover from it. Not every application embedding this
library has an autosave of its own to fall back on. A library that can
unilaterally end its host's process (poison pill) leaves that host unable
to offer any guarantee of its own.

I am asking whether you would accept a settable fatal-error handler, so an
embedding application gets a chance to save the user's work before the
process ends. Default behavior would be unchanged. I am happy to do the
work.

What already works here

Worth saying first, because it shapes the request: the diagnostics in this
area are good. Slvs_Tangent() dispatches correctly on entity type, works
out the other endpoint by testing which points actually coincide, and
when it refuses, it says exactly why and what to do instead:

The tangent arc and line segment must share an endpoint. Constrain them
with Constrain -> On Point before constraining tangent.

That is a better error message than most libraries manage. The problem is
purely that it goes to stderr (more on this later) and is followed by
abort(), so the one program that could act on it, the host, never
receives it.

Where Slvs_Tangent() validates, it produces that
excellent sentence and then aborts, so I cannot use it. Where a wrong
entity type slips past validation into the kernel instead, I get
Cannot find handle from dsc.h and then it aborts, so I have nothing to
use. Both paths end at the same abort(), and the good diagnostic is
wasted for exactly the same reason the bad one is unhelpful.

Likewise #1542's stated rationale, "so that users of the solver could use
the solution and deal with the redundant constraints in any way they
like", is exactly the principle this request is built on. I am asking for
the same courtesy one layer down.

Why this is different from the application case

CONTRIBUTING.md explains the current design:

Unlike the standard assert function, the ssassert function is always
enabled, even in release builds. It is more valuable to discover a bug
through a crash than to silently generate incorrect results, and crashes
do not result in losing more than a few minutes of work thanks to the
autosave feature.

I agree with that reasoning for SolveSpace itself. The point I would raise
is that its final clause is what makes it safe, and a foreign app has no
SolveSpace autosave. The user of my application loses their session, not
a few minutes.

CONTRIBUTING.md also describes ssassert as the tool for internal
invariants:

To ensure that internal invariants hold, the ssassert function is used

The calls in question are not internal invariant checks. They are
validation of arguments supplied by an external caller across a public C
ABI, which is a different category.

SolveSpace already treats this as a host decision

This is not a new idea in this codebase. Platform::FatalError is
implemented separately for every platform, and no two do the same thing:

The reason there are six implementations is that how a fatal error should
be surfaced depends on the host. A desktop app raises a dialog; a macOS
build hands it to the crash reporter; a browser build routes it to the
console.

The library is the one case where the host cannot be known at compile
time. It is not that fprintf(stderr) was judged right for embedders; it
is that a compiled-in choice is the only kind available today. A settable
handler is the same idea, expressed at the only point where it can be: at
run time.

This matters beyond recovery. stderr belongs to the application. A
Windows GUI host usually has no console attached, so the message is
discarded outright. guiwin clearly knows this, hence the dialog. The
Python binding and the wasm build both link this same library build, so a
Python caller currently gets a process abort with no exception, no
traceback, and a string written somewhere they may never see.

Incidentally, guiwin already guards re-entry with a handlingFatalError
flag, which is the same precaution suggested for the handler below.

What I am not proposing

Not exceptions. CONTRIBUTING.md rules those out for a concrete reason:

Exceptions are not used primarily because SolveSpace's testsuite uses
measurement of branch coverage ... Every function call with exceptions
enabled introduces a branch, making branch coverage measurement useless.

That objection stands, and an exception crossing an extern "C" boundary
would also break the existing lib.pyx and jslib.cpp bindings.

Precedent

Three existing decisions suggest this direction is compatible with how the
project already works:

• Export: warn instead of crashing on freehand/zigzag stipple #1752 (merged) replaced exactly this pattern in the exporter:
  "Replace the ssassert() with a dbp() warning and fall through to export
  the line as continuous, so a valid file is produced instead of
  aborting." An unsupported input became a warning plus a sensible
  fallback rather than an abort.

• Expose the solver's SolveResult::REDUNDANT_OKAY result in the slvs library #1542 (merged) added a new result code to this very API,
  SLVS_RESULT_REDUNDANT_OKAY, with the rationale of "giving a
  successful solve a distinct return value, so that users of the solver
  could use the solution and deal with the redundant constraints in any
  way they like." Extending the result set is established practice, and
  the stated principle (inform the lib caller and let them decide) is the
  same one here.

• Constraint solver library crashing on parallel (and related) constraints #1379 (closed as completed) was a library user hitting an abort
  through the C API on SLVS_C_PARALLEL / SLVS_C_SAME_ORIENTATION /
  SLVS_C_CUBIC_LINE_TANGENT in 3D. It was treated as a bug and fixed
  rather than as intended behavior.

The principle

To be clear about scope: this is not a request to stop aborting. It is a
request that the program which owns the process gets a say in what happens
to it.

Today a user app has no say at all, because abort() does not return.
There is no point at which the host can log the fault, save the document,
or tell its user anything. That is the gap, and the smallest thing that
closes it is a hook called on the way out.

Things I tried first, so you do not have to suggest them

I would rather not ask for an API change if a caller can solve this alone.
I could not, and the reason is structural rather than a matter of effort.

Overriding the symbol at link time. Platform::FatalError is exported
from libslvs.so with default visibility, so defining it in the
application looks like it should interpose. It does not: the library still
runs its own copy and aborts.

It fails, and here is the proof:

$ readelf -r /usr/lib/x86_64-linux-gnu/libslvs.so.3.2 | grep -c FatalError
0
$ objdump -d /usr/lib/x86_64-linux-gnu/libslvs.so.3.2 | grep -c 'FatalError@plt'
0

There are no dynamic relocations and no PLT entries for it. FatalError
is defined in lib.cpp alongside most of its callers, so those calls are
bound directly at link time and the dynamic linker is never consulted.
There is nothing to interpose, which is also why LD_PRELOAD does not
help: I tried that too, with a preload object exporting the correctly
mangled symbol, and the library ignored it.

Catching SIGABRT. This does work, and it is what my application
currently does. Two things it cannot give me: the handler runs in signal
context, with the async-signal-safety limits that implies for anything as
involved as saving a document; and the diagnostic never arrives, because
the message went to stderr before abort() was called. I can tell that
something fatal happened, but not what.

That last point is the gap. The library already knows exactly what went
wrong and has written a clear sentence about it. There is currently no way
for my host app to receive that sentence.

Suggested mechanism: a settable fatal-error handler

The smallest change that addresses the harm is for the library to hold an
error handler that the embedder can install, consult it when it is about
to terminate, and let it say what should happen:

typedef enum {
    /* Terminate, exactly as today. */
    SLVS_FATAL_ABORT        = 0,
    /* Request: unwind and report to the caller where possible. */
    SLVS_FATAL_RETURN_ERROR = 1
} Slvs_FatalErrorAction;

typedef Slvs_FatalErrorAction (*Slvs_FatalErrorHandler)(
        const char *message, void *context);

/* Installs a handler consulted when the library is about to terminate.
 * Passing NULL restores the default. With no handler installed the
 * library aborts, exactly as it does today. */
DLL void Slvs_SetFatalErrorHandler(Slvs_FatalErrorHandler handler,
                                   void *context);

SLVS_FATAL_ABORT is deliberately zero, so the default, the no-handler
case, and a handler that returns nothing meaningful all land on current
behavior.

Returning SLVS_FATAL_RETURN_ERROR is a request rather than a
guarantee. The library would honor it at sites where it can unwind
cleanly and report through an existing result channel, and fall back to
aborting where it cannot, documented per site. That keeps the first change
small while making it possible to widen coverage later without a second
API break, which is the only reason the enum is worth having now rather
than adding it in a year.

Why this shape:

• One site, not 108, and it is already library-only.
  src/slvs/CMakeLists.txt builds the library from lib.cpp with
  -DLIBRARY; it does not compile platform/guinone.cpp, and lib.cpp
  provides its own Platform::FatalError at line 16. The hook goes there.
  Every abort path in the library funnels through it, including any not
  enumerated here.
• SolveSpace itself cannot be affected. The application compiles a
  different FatalError, so this change is invisible to it. With no
  handler installed the library aborts exactly as it does today, so
  existing library users who do not opt in see no change either.
• No exceptions anywhere. The handler is a plain C function pointer,
  so the branch-coverage objection in CONTRIBUTING.md does not apply,
  and nothing unwinds across the extern "C" boundary.
• No reclassification required. It does not ask you to decide which of
  the 108 exit points are caller errors and which are internal invariants,
  a judgment call that would need review site by site.

On the SLVS_FATAL_ABORT path (the default, and the only one that has to
work for this to be useful), nothing can leak or be corrupted, because
control flow is unchanged: the handler runs, returns, and the process
aborts as before. No early return, no allocation lifetime changes, no
cleanup skipped. That is the whole of the first change.

SLVS_FATAL_RETURN_ERROR is where care is needed, and it is why I would
rather it start narrow: an early return has to run the same teardown the
normal path does, or it leaves state behind in a global. The section below
sets out what that involves.

A re-entrancy guard is worth having either way, so that a fault inside the
handler cannot recurse. That is standard for crash handlers.

The handler pointer is global state, so the expectation would be that it
is installed once during startup, before any solving begins, and not
swapped while a solve is in progress. That is how an embedder would use it
in practice, and it keeps the change clear of the OpenMP parallelism in
the kernel.

The handler would be documented as running in an already-fatal context,
with the same constraints any crash handler has: do the minimum, do not
assume library state is usable, do not expect to return to normal
operation. Its job is last rites (flush a log, emergency-save the
document, tell the user what happened), and then the process ends as
before.

This is the same courtesy SolveSpace's own autosave extends to its users.
It simply makes it available to user apps built on the library.

A longer-term option: returning errors for caller input

This is what honoring SLVS_FATAL_RETURN_ERROR would mean in practice,
and where I would expect it to start. A subset of these sites are argument
validation rather than internal invariant checks, and could report an
error instead of terminating at all. Slvs_Tangent() is a clear example:
it already detects the problem and writes a helpful message before
aborting.

The API has channels for this already. Slvs_SolveResult already carries
{ result, dof, nbad }, and #1542 set the precedent for adding a code to
it. The construction helpers return structs with an h handle, so
h == 0 would be an idiomatic "rejected" signal, ABI-compatible with
callers that ignore the return value.

One caveat if this is ever pursued, which is a real argument for keeping
it separate: Slvs_Solve() ends by running bad.Clear(), SYS.Clear(),
SK.param.Clear(), SK.entity.Clear(), SK.constraint.Clear() and
Platform::FreeAllTemporary(). Any early return would have to run all of
them. Because SK is a file-scope global in lib.cpp, skipping that
teardown would not merely leak; it would leave stale entities visible to
the next call. That is a correctness hazard, not just a memory one, and
it needs care that the handler above does not.

I am not proposing this as part of the same change. The handler is useful
with SLVS_FATAL_ABORT alone, since it already closes the gap that costs
users their work, and it is much smaller to review. Widening
RETURN_ERROR coverage is a follow-up worth discussing only if the first
is welcome.

Environment

• libslvs built from master at 952c11c (2026-09-01), Ubuntu 24.04, GCC 13
• Embedding application: a Qt/OpenCASCADE CAD program using libslvs for 2D
  sketch constraints

0001-slvs-fatal-error-handler.patch
slvs_abort_repro.c