diff --git a/CHANGELOG.md b/CHANGELOG.md index ab841d2de..1338f8c03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,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 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..0a278ecb5 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.7.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():