From 2370a94f602d3f4aedd498bd4618c3bff6a54b10 Mon Sep 17 00:00:00 2001 From: Mohammed Ghannam Date: Mon, 17 Nov 2025 09:34:41 +0100 Subject: [PATCH 1/3] Manage benders subproblems memory in the master scip instance --- CHANGELOG.md | 8 ++++ src/pyscipopt/scip.pxd | 2 + src/pyscipopt/scip.pxi | 68 +++++++++++++++++++++++++-------- tests/test_benders.py | 5 --- tests/test_customizedbenders.py | 10 ----- 5 files changed, 63 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab841d2de..0957532a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ ## Unreleased ### Added +### Fixed +- Fixed segmentation fault during Benders decomposition cleanup caused by double-free bug +### Changed +- Benders subproblem memory is now automatically managed by the master Model - `freeBendersSubproblems()` is deprecated and no longer needed +### Removed + +## 5.6.0 - 2025.11.01 +### Added - Added possibility of having variables in exponent. - Added basic type stubs to help with IDE autocompletion and type checking. - MatrixVariable comparisons (<=, >=, ==) now support numpy's broadcast feature. diff --git a/src/pyscipopt/scip.pxd b/src/pyscipopt/scip.pxd index 7d99e4715..a37e3c8df 100644 --- a/src/pyscipopt/scip.pxd +++ b/src/pyscipopt/scip.pxd @@ -2119,6 +2119,8 @@ cdef class Model: cdef _modelvars # used to keep track of the number of event handlers generated cdef int _generated_event_handlers_count + # store references to Benders subproblem Models for proper cleanup + cdef _benders_subproblems @staticmethod cdef create(SCIP* scip) diff --git a/src/pyscipopt/scip.pxi b/src/pyscipopt/scip.pxi index 5f2d864c5..c982e795c 100644 --- a/src/pyscipopt/scip.pxi +++ b/src/pyscipopt/scip.pxi @@ -2597,6 +2597,7 @@ cdef class Model: self._freescip = True self._modelvars = {} self._generated_event_handlers_count = 0 + self._benders_subproblems = [] # Keep references to Benders subproblem Models if not createscip: # if no SCIP instance should be created, then an empty Model object is created. @@ -2668,10 +2669,29 @@ cdef class Model: self.includeEventhdlr(event_handler, name, description) def __dealloc__(self): + # Declare all C variables at the beginning for Cython compatibility + cdef SCIP_BENDERS** benders + cdef int nbenders + cdef int nsubproblems + cdef int i, j + # call C function directly, because we can no longer call this object's methods, according to # http://docs.cython.org/src/reference/extension_types.html#finalization-dealloc if self._scip is not NULL and self._freescip and PY_SCIP_CALL: - PY_SCIP_CALL( SCIPfree(&self._scip) ) + # Free Benders subproblems before freeing the main SCIP instance + if self._benders_subproblems: + benders = SCIPgetBenders(self._scip) + nbenders = SCIPgetNActiveBenders(self._scip) + + for i in range(nbenders): + nsubproblems = SCIPbendersGetNSubproblems(benders[i]) + for j in range(nsubproblems): + PY_SCIP_CALL(SCIPfreeBendersSubproblem(self._scip, benders[i], j)) + + # Clear the references to allow Python GC to clean up the Model objects + self._benders_subproblems = [] + + PY_SCIP_CALL( SCIPfree(&self._scip) ) def __hash__(self): return hash(self._scip) @@ -2700,6 +2720,7 @@ cdef class Model: model = Model(createscip=False) model._scip = scip model._bestSol = Solution.create(scip, SCIPgetBestSol(scip)) + model._benders_subproblems = [] # Initialize Benders subproblems list return model @property @@ -8353,6 +8374,21 @@ cdef class Model: PY_SCIP_CALL(SCIPcreateBendersDefault(self._scip, subprobs, nsubproblems)) benders = SCIPfindBenders(self._scip, "default") + # Free the temporary array (SCIPcreateBendersDefault copies the pointers internally) + free(subprobs) + + # Store references to subproblem Models and transfer ownership to master. + # The master will free the Benders subproblems in its __dealloc__ method. + # Set _freescip = False on the subproblem Model(s) to prevent double-free + # if they are deallocated before the master. + if isdict: + self._benders_subproblems = list(subproblems.values()) + for subprob in self._benders_subproblems: + (subprob)._freescip = False + else: + self._benders_subproblems = [subproblems] + (subproblems)._freescip = False + # activating the Benders' decomposition constraint handlers self.setBoolParam("constraints/benderslp/active", True) self.setBoolParam("constraints/benders/active", True) @@ -8383,20 +8419,22 @@ cdef class Model: benders[i], self._bestSol.sol, j, &infeasible, solvecip, NULL)) def freeBendersSubproblems(self): - """Calls the free subproblem function for the Benders' decomposition. - This will free all subproblems for all decompositions. """ - cdef SCIP_BENDERS** benders = SCIPgetBenders(self._scip) - cdef int nbenders = SCIPgetNActiveBenders(self._scip) - cdef int nsubproblems - cdef int i - cdef int j - - # solving all subproblems from all Benders' decompositions - for i in range(nbenders): - nsubproblems = SCIPbendersGetNSubproblems(benders[i]) - for j in range(nsubproblems): - PY_SCIP_CALL(SCIPfreeBendersSubproblem(self._scip, benders[i], - j)) + """Deprecated: This method is no longer needed. + + Benders subproblems are now automatically managed and freed by the master + Model when it is deallocated. Calling this method explicitly has no effect. + + .. deprecated:: 5.2.0 + This method is deprecated and will be removed in a future version. + Subproblem memory management is now handled automatically. + """ + warnings.warn( + "freeBendersSubproblems() is deprecated and no longer needed. " + "Benders subproblems are automatically freed when the master Model " + "is deallocated.", + DeprecationWarning, + stacklevel=2 + ) def updateBendersLowerbounds(self, lowerbounds, Benders benders=None): """ diff --git a/tests/test_benders.py b/tests/test_benders.py index 2636b4c5e..e8e05051d 100644 --- a/tests/test_benders.py +++ b/tests/test_benders.py @@ -101,9 +101,4 @@ def test_flpbenders(): master.printStatistics() - # since computeBestSolSubproblems() was called above, we need to free the - # subproblems. This must happen after the solution is extracted, otherwise - # the solution will be lost - master.freeBendersSubproblems() - assert 5.61e+03 - 10e-6 < master.getObjVal() < 5.61e+03 + 10e-6 diff --git a/tests/test_customizedbenders.py b/tests/test_customizedbenders.py index aaa461296..6a64bc3af 100644 --- a/tests/test_customizedbenders.py +++ b/tests/test_customizedbenders.py @@ -228,11 +228,6 @@ def flpbenders_defcuts_test(): master.printStatistics() - # since the subproblems were setup and then solved, we need to free the - # subproblems. This must happen after the solution is extracted, otherwise - # the solution will be lost - master.freeBendersSubproblems() - return master.getObjVal() def flpbenders_customcuts_test(): @@ -278,11 +273,6 @@ def flpbenders_customcuts_test(): master.printStatistics() - # since the subproblems were setup and then solved, we need to free the - # subproblems. This must happen after the solution is extracted, otherwise - # the solution will be lost - master.freeBendersSubproblems() - return master.getObjVal() def flp_test(): From 8cb597c1f505d8e34a9aef9216d8a5aa91c49375 Mon Sep 17 00:00:00 2001 From: Mohammed Ghannam Date: Mon, 17 Nov 2025 10:07:49 +0100 Subject: [PATCH 2/3] Update src/pyscipopt/scip.pxi MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: João Dionísio <57299939+Joao-Dionisio@users.noreply.github.com> --- src/pyscipopt/scip.pxi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pyscipopt/scip.pxi b/src/pyscipopt/scip.pxi index c982e795c..0a278ecb5 100644 --- a/src/pyscipopt/scip.pxi +++ b/src/pyscipopt/scip.pxi @@ -8424,7 +8424,7 @@ cdef class Model: Benders subproblems are now automatically managed and freed by the master Model when it is deallocated. Calling this method explicitly has no effect. - .. deprecated:: 5.2.0 + .. deprecated:: 5.7.0 This method is deprecated and will be removed in a future version. Subproblem memory management is now handled automatically. """ From b981cedb72a5f030c65c3205937923379e357010 Mon Sep 17 00:00:00 2001 From: Mohammed Ghannam Date: Mon, 17 Nov 2025 10:11:38 +0100 Subject: [PATCH 3/3] Fix changelog --- CHANGELOG.md | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0957532a8..1338f8c03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,14 +2,6 @@ ## Unreleased ### Added -### Fixed -- Fixed segmentation fault during Benders decomposition cleanup caused by double-free bug -### Changed -- Benders subproblem memory is now automatically managed by the master Model - `freeBendersSubproblems()` is deprecated and no longer needed -### Removed - -## 5.6.0 - 2025.11.01 -### Added - Added possibility of having variables in exponent. - Added basic type stubs to help with IDE autocompletion and type checking. - MatrixVariable comparisons (<=, >=, ==) now support numpy's broadcast feature. @@ -23,12 +15,15 @@ - Implemented all binary operations between MatrixExpr and GenExpr - Fixed the type of @ matrix operation result from MatrixVariable to MatrixExpr. - Fixed the case for returning None from the nodeselect callback in Node Selector plugins. +- Fixed segmentation fault during Benders decomposition cleanup caused by double-free bug ### Changed - Add package extras for test dependencies in `pyproject.toml` - Speed up MatrixVariable.sum(axis=None) via quicksum - MatrixVariable now supports comparison with Expr +- Benders subproblem memory is now automatically managed by the master Model - `freeBendersSubproblems()` is deprecated and no longer needed ### Removed + ## 5.6.0 - 2025.08.26 ### Added - More support for AND-Constraints