fix(solvers): dispose the solver model before its env on close

doc/release_notes.rst

@@ -54,7 +54,11 @@ Upcoming Version
 **Bug fixes**
 
 * A multi-key ``groupby`` now returns its groups sorted by key tuple, like the single-key path. The key combinations were numbered by iterating a ``set``, so the group order was arbitrary and changed between processes with ``PYTHONHASHSEED``.
+* ``Solver.close()`` no longer leaves dangling native handles behind. The solver model is now dropped before the environment that owns it, instead of after. And the COPT and MindOpt file interfaces no longer hand back a model they already disposed: after a file-based COPT or MindOpt solve, ``model.solver_model`` is ``None`` rather than a handle into freed memory. (`#899 <https://github.com/PyPSA/linopy/pull/899>`__)
 
+**Breaking Changes**
+
+* A solver is no longer released automatically when the model is garbage-collected, withdrawing a guarantee announced in 0.8.0. The finalizer that did so ran mid-collection and could tear down native state inside an unrelated call stack, aborting the interpreter. Release the solver explicitly instead, with ``model.solver.close()``, ``model.solver = None``, or ``contextlib.closing(model.solver)``. A new ``solve()`` still closes the solver it replaces, so a re-solve loop needs no change. (`#899 <https://github.com/PyPSA/linopy/pull/899>`__)
 
 Version 0.9.1
 -------------

linopy/model.py

@@ -2011,7 +2011,8 @@ def solve(
         means the license remains acquired until the solver is released:
         call ``model.solver.close()`` (or assign ``model.solver = None``)
         to free it explicitly. It is also released on the next ``solve()``
-        call and when the model is garbage-collected.
+        call, but not by garbage collection: a model that merely goes out of
+        scope leaves its solver open.
         """
         if mock_solve:
             return self._mock_solve(

linopy/solvers.py

@@ -1119,21 +1119,19 @@ def close(self) -> None:
         user-supplied environment is left untouched.
 
         Idempotent, and called automatically when a new ``solve()`` replaces
-        this solver, when ``model.solver`` is reassigned (e.g. to ``None``),
-        and on garbage collection. After closing, post-solve introspection
+        this solver and when ``model.solver`` is reassigned (e.g. to ``None``).
+        Deliberately not called from a finalizer: a solver is only reachable in
+        a ``Model``/``Solver`` reference cycle, so that would tear down native
+        handles mid-collection. After closing, post-solve introspection
         (``solver_model``, ``compute_infeasibilities()``) and persistent
         re-solves are no longer available.
         """
+        self.solver_model = None
         if self._env_stack is not None:
             self._env_stack.close()
         self.env = None
-        self.solver_model = None
         self._env_stack = None
 
-    def __del__(self) -> None:
-        with contextlib.suppress(Exception):
-            self.close()
-
     def __getstate__(self) -> dict[str, Any]:
         drop = {"solver_model", "env", "_env_stack", "snapshot", "_lock"}
         return {k: v for k, v in self.__dict__.items() if k not in drop}
@@ -3998,7 +3996,7 @@ def get_solver_solution() -> Solution:
             solution = maybe_adjust_objective_sign(solution, io_api, sense)
 
             self.io_api = io_api
-            return self._make_result(status, solution, solver_model=m)
+            return self._make_result(status, solution)
         finally:
             env_.close()
 
@@ -4136,7 +4134,7 @@ def get_solver_solution() -> Solution:
             solution = maybe_adjust_objective_sign(solution, io_api, sense)
 
             self.io_api = io_api
-            return self._make_result(status, solution, solver_model=m)
+            return self._make_result(status, solution)
         finally:
             if m is not None:
                 m.dispose()

test/test_solvers.py

@@ -204,6 +204,42 @@ def test_gurobi_env_persists_after_solve(simple_model: Model) -> None:
     assert isinstance(simple_model.solver_model.NumVars, int)
 
 
+@pytest.mark.skipif(
+    "copt" not in set(solvers.licensed_solvers), reason="COPT is not installed"
+)
+def test_copt_closes_env_per_solve(
+    simple_model: Model, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """
+    The file interface closes its environment before returning, so it must not
+    hand back a solver model that outlives it. Holding the environment open
+    instead would leak it: nothing closes a solver that is merely dropped.
+    """
+    import coptpy
+
+    closes: list[object] = []
+    original = coptpy.Envr.close
+
+    def close(self: object) -> None:
+        closes.append(self)
+        original(self)
+
+    monkeypatch.setattr(coptpy.Envr, "close", close)
+
+    simple_model.solve("copt")
+
+    assert simple_model.solver_model is None
+    assert closes
+
+
+def test_solver_defines_no_finalizer() -> None:
+    """
+    A solver is only reachable in a Model/Solver reference cycle, so a
+    finalizer would tear down native handles mid-collection.
+    """
+    assert not hasattr(solvers.Solver, "__del__")
+
+
 @pytest.mark.parametrize("solver", sorted(set(solvers.licensed_solvers)))
 def test_solver_close_releases_state(simple_model: Model, solver: str) -> None:
     simple_model.solve(solver)