Fix free transform segfault

[Joao-Dionisio]

Fix #604 , the oldest issue in the repo. Basically, after freeTransform(), VsCode would try to access freed memory, via Python's __repr__. This fix caches the names of variables and constraints to prevent the problem.

[DominikKamp]

This looks like an unnecessary overhead. Should the variable not be captured to avoid dangling pointers? Or maybe just reset the variable pointers in free methods and check this state in __get__() and everywhere else.

[Joao-Dionisio]

That seems like a lot more work. And at least Claude doesn't seem to enthusiastic. Regarding variable capture and release:

when Python garbage collects Variables after the Model is freed, SCIPreleaseVar(self._scip, ...) fails because the SCIP instance is already gone. This caused errors during cleanup. Could potentially be solved by storing a reference to the Model (not raw SCIP*) so the Model stays alive while Variables exist, but that creates other complexities (circular references, preventing Model cleanup).

It seems like a lot of work will be needed. How do you feel about having this short-term solution and keeping the proper fix in mind for later? Otherwise, the bug will remain there for a lot longer, I don't have the bandwith.

[DominikKamp]

The model destructor could release the variables before freeing the model, but this is a conventional change. Simple fix would be to declare the transformed variables as freed by setting the stored variable pointer to NULL in freeTransform() before restricting the model variables to the original variables. Then it is just not allowed to call methods on these freed variables (same as currently), which could be safeguarded by additional checks, so the described issue is just that PySCIPOpt uses an invalid memory address.

[Joao-Dionisio]

What about now @DominikKamp ?

[Copilot]

Pull request overview

This pull request addresses issue #604, which involved segmentation faults when accessing freed memory through Python's __repr__ method after calling freeTransform() or when the Model is destroyed. The fix adds NULL pointer checks in the __repr__ methods and implements a tracking mechanism to invalidate pointers when objects are freed.

Changes:

• Added NULL pointer checks in Variable.__repr__() and Constraint.__repr__() to return special strings for freed objects
• Implemented tracking for constraints in _modelconss dictionary (similar to existing _modelvars for variables)
• Added pointer invalidation logic in Model.__dealloc__() and Model.freeTransform() to set pointers to NULL before freeing

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
tests/test_model.py	Added two test cases to verify that repr() works correctly on freed variables and constraints after freeTransform() and Model destruction
src/pyscipopt/scip.pxi	Implemented NULL checks in __repr__ methods, added _modelconss tracking, pointer invalidation in cleanup methods, and tracking in constraint/variable creation and retrieval methods
src/pyscipopt/scip.pxd	Added _modelconss field declaration to Model class
CHANGELOG.md	Added entry documenting the segfault fix

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Joao-Dionisio]

@DominikKamp can it be merged, then?

[DominikKamp]

Otherwise, this looks fine, probably NULL handlings should be added to more methods but using deleted objects is just not supported, however, the master branch should be fixed before merging anything.

[Joao-Dionisio]

however, the master branch should be fixed before merging anything.

This is a problem with a recent merge not being compatible with numpy 1.16. It's not really relevant (numpy 1.16 is 7 years old), and is actively being worked on in #1164