Exception safety

[ConradDrescher]

Various exception safety and memory management related fixes

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[zayenz]

Thank you to @ConradDrescher for opening PR 211, and to Kris Coester and Alexander Shepil for the patch commits. The PR found real problems in Gecode's exception-safety and memory-management paths, especially around failed cloning, IntSetObject allocation, dispose-notice registration, and MiniModel expression construction.

I am going to close this PR as superseded by PR #212 rather than merge it directly. The replacement branch reimplements the accepted parts with deterministic fault-injection tests and sanitizer validation. That review also found a few places where the final ownership rule should be different from the one encoded in this pr.

This PR provided the starting observations, and the replacement branch notes the relevant authors from this pr in the changelog and in source-file contributor lists where the final change is substantially based on the submitted work.

Area	What this pr identified	What PR #212 does	Why the final branch differs
Clone exception safety	Failed cloning can leave forwarding links and clone-owned structures in a bad state.	Reimplements clone recovery as a transaction covering actors, advisors, local objects, variable implementations, disposal metadata, and derived Space construction.	The invariant is right, but the final branch needed deterministic failures at each ownership boundary and generated-file changes synchronized with misc/genvarimp.py.
Destructor-visible clone fields	Partial clone construction can reach destructors that read fields before normal construction has finished.	Initializes clone/destructor-visible state early and tests disposal-array failure and unfinished clone destruction.	This is one of the places where the replacement branch keeps the PR 211 idea very directly, then extends the coverage.
Recovery state	PR 211 introduced an isFullyConstructed style guard and made recovery more explicit.	Keeps the state-machine idea but ties recovery to the clone transaction and keeps clone recovery non-virtual.	Recovery is an internal cleanup contract. The final branch avoids exposing a new virtual extension point for it.
Advisor cancellation	Advisor cancellation is unsafe while an unfinished clone is being destroyed.	Skips unsafe cancellation during unfinished clone cleanup, handles marked advisor links without typed casts, and covers partial advisor-copy failure.	UBSan and fault injection pointed to a slightly different implementation shape than the PR patch.
Propagation statistics	PR 211 moved propagation-stat accounting behind a virtual hook.	Does not include this change.	No failing clone or allocation test required it, and it adds dispatch to a propagation hot path. It should be a separate change if the project wants that extension point.
IntSetObject allocation	Range allocation failure can leak or corrupt a partially constructed object.	Keeps the allocation-safety fix, initializes cleanup state before range allocation, preserves throw;, and adds fault tests.	The accepted invariant is the same, but the replacement branch adds tests and later UBSan fixes for full-range width arithmetic.
Space::ap_notice_dispose	Allocation failure during dispose-notice registration can lose ownership.	Adds a regression for registration failure and avoids disposing actors that were never registered.	The test made the ownership boundary clearer: before successful registration, the space does not own the actor. A direct port would have disposed an unregistered actor.
MiniModel expressions	Some BoolExpr, IntExpr, and nonlinear arithmetic construction paths leak on allocation failure.	Reworks partial-construction cleanup while preserving the public default LinIntExpr zero-expression invariant.	PR 211 found the right area. The final branch needed a narrower internal no-node form instead of changing the public default expression semantics.
Locking changes	PR 211 narrows several locking scopes in tracing and memory-management-adjacent code.	Does not import the broad locking commit as a unit. Adds narrower TSan-driven fixes for search command state, path stealing, stop state, PBS lifetime, restart-stop state, shared statistics, and no-good accounting.	The locking commit is broader than the exception-safety work. The replacement branch only takes synchronization changes with observed sanitizer failures behind them.
Tests and validation	PR 211 does not include regression tests or sanitizer configuration.	Adds opt-in deterministic fault injection and validates the final branch with targeted ASan, UBSan, and TSan runs.	The failures are mostly transactional and allocation-boundary dependent. They needed tests that can force the exact failing allocation or copy step.

In summary, this pr identified valuable defects, and several of its ideas are present in the final branch. The replacement branch is shaped differently because the tests made the ownership and recovery contracts explicit, and in a few cases those contracts disagreed with the submitted patch shape.