Skip to content

Commit 5366af9

Browse files
committed
Clarify plugin lifecycle and hide eventhdlr bookkeeping
1 parent fae41e8 commit 5366af9

6 files changed

Lines changed: 36 additions & 19 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
### Fixed
88
- Removed `Py_INCREF`/`Py_DECREF` on `Model` in `catchEvent`/`dropEvent` that caused memory leak for imbalanced usage
99
- Used `getIndex()` instead of `ptr()` for sorting nonlinear expression terms to avoid nondeterministic behavior
10-
- Replaced `weakref.proxy` with strong references for plugin `self.model`, fixing `ReferenceError` during cleanup callbacks (#1193)
10+
- Plugins now hold strong references to their `Model` instead of `weakref.proxy`, fixing `ReferenceError` during cleanup callbacks (#1193)
1111
### Changed
1212
- Speed up `constant * Expr` via C-level API
1313
### Removed

src/pyscipopt/event.pxi

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
cdef class Eventhdlr:
44
cdef public Model model
55
cdef public str name
6-
cdef public list _caught_events
6+
# eventtypes caught via `Model.catchEvent`, auto-dropped on `eventexit`.
7+
cdef list _caught_events
78

89
def __cinit__(self):
910
self._caught_events = []

src/pyscipopt/scip.pxd

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2251,7 +2251,11 @@ cdef class Model:
22512251
cdef int _generated_event_handlers_count
22522252
# store references to Benders subproblem Models for proper cleanup
22532253
cdef _benders_subproblems
2254-
# store references to plugins for the Model <-> Plugin reference cycle
2254+
# Strong references to every included plugin. Each plugin in turn holds a
2255+
# strong reference to the Model via `self.model`, so both stay alive until
2256+
# SCIP teardown callbacks (consfree, eventexit, ...) have finished running.
2257+
# The resulting cycle is broken by `Model.__del__` or by `Model.free()`.
2258+
# See `Model.free` for the full lifecycle policy.
22552259
cdef _plugins
22562260
# store iis, if found
22572261
cdef SCIP_IIS* _iis

src/pyscipopt/scip.pxi

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2899,19 +2899,16 @@ cdef class Model:
28992899
self.includeEventhdlr(event_handler, name, description)
29002900

29012901
def __del__(self):
2902-
"""Free SCIP before the cyclic GC clears Python references.
2902+
"""Free SCIP when the last external reference to the Model is dropped.
29032903
2904-
__del__ (tp_finalize) runs before tp_clear, so _plugins and other
2905-
Python attributes are still intact. This lets SCIPfree call plugin
2906-
callbacks (consfree, eventexit, etc.) safely. The GC handles breaking
2907-
the circular references afterwards via tp_clear.
2904+
Runs before the garbage collector clears the Model's attributes, so
2905+
plugin callbacks can still reach a live ``self.model``. See
2906+
``Model.free`` for the full lifecycle policy.
29082907
"""
29092908
self._free_scip_instance()
29102909

29112910
def __dealloc__(self):
2912-
# Safety net: if __del__ didn't run (e.g. interpreter shutdown),
2913-
# free SCIP here. Plugin callbacks may not work at this point since
2914-
# tp_clear may have already cleared _plugins.
2911+
"""Fallback SCIP cleanup if ``__del__`` did not already free the instance."""
29152912
if self._scip is not NULL and self._freescip:
29162913
SCIPfree(&self._scip)
29172914

@@ -3089,11 +3086,28 @@ cdef class Model:
30893086
PY_SCIP_CALL(SCIPfreeProb(self._scip))
30903087

30913088
def free(self):
3092-
"""Explicitly free SCIP instance and break circular references with plugins.
3093-
3094-
This method should be called when you want to ensure deterministic cleanup.
3095-
All SCIP callbacks (consfree, eventexit, etc.) are executed during this call.
3096-
After calling this method, the Model object should not be used anymore.
3089+
"""Explicitly free the SCIP instance and release plugin references.
3090+
3091+
Every included plugin holds a strong reference to its Model via
3092+
``self.model``, and the Model holds strong references to every plugin
3093+
via ``Model._plugins``. This cycle is intentional: it keeps both sides
3094+
alive during SCIP teardown so callbacks such as ``eventexit`` and
3095+
``consfree`` can safely reach ``self.model``.
3096+
3097+
The cycle is broken in one of two ways:
3098+
3099+
* Implicitly, via ``Model.__del__`` when the last external reference to
3100+
the Model is dropped. ``__del__`` runs before the garbage collector
3101+
(GC) clears attributes, so callbacks still see a live ``self.model``.
3102+
Timing is at the GC's discretion.
3103+
* Explicitly, by calling this method. SCIP is freed immediately,
3104+
teardown callbacks fire before ``free`` returns, and the plugin list
3105+
is cleared. Use this when deterministic cleanup is required. After
3106+
``free`` the Model must not be used further.
3107+
3108+
The solve itself is not affected by which path runs. ``free`` only
3109+
controls when memory and resources are released, and when teardown
3110+
callbacks execute.
30973111
"""
30983112
self._free_scip_instance()
30993113

src/pyscipopt/scip.pyi

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,6 @@ class Event:
315315
class Eventhdlr:
316316
model: Incomplete
317317
name: Incomplete
318-
_caught_events: list
319318
def __init__(self) -> None: ...
320319
def eventcopy(self) -> Incomplete: ...
321320
def eventdelete(self) -> Incomplete: ...

tests/test_event.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,7 @@ def eventexec(self, event):
200200
with pytest.raises(Exception):
201201
m.optimize()
202202

203-
del m, ev, v
204-
gc.collect()
203+
m.free()
205204

206205
def test_missing_dropEvent_cleanup():
207206
"""Model is garbage collected even when dropEvent is not called in eventexit."""

0 commit comments

Comments
 (0)