From f4507e25663cb292ec584e284cbdfe2c84a1358a Mon Sep 17 00:00:00 2001 From: Joao-Dionisio Date: Thu, 19 Feb 2026 13:25:05 +0000 Subject: [PATCH 1/7] Remove spurious Py_INCREF/DECREF on Model in catchEvent/dropEvent --- src/pyscipopt/scip.pxi | 2 -- tests/test_event.py | 39 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/pyscipopt/scip.pxi b/src/pyscipopt/scip.pxi index 9c21942bd..02b1ede07 100644 --- a/src/pyscipopt/scip.pxi +++ b/src/pyscipopt/scip.pxi @@ -11268,7 +11268,6 @@ cdef class Model: else: raise Warning("event handler not found") - Py_INCREF(self) PY_SCIP_CALL(SCIPcatchEvent(self._scip, eventtype, _eventhdlr, NULL, NULL)) def dropEvent(self, eventtype, Eventhdlr eventhdlr): @@ -11288,7 +11287,6 @@ cdef class Model: else: raise Warning("event handler not found") - Py_DECREF(self) PY_SCIP_CALL(SCIPdropEvent(self._scip, eventtype, _eventhdlr, NULL, -1)) def catchVarEvent(self, Variable var, eventtype, Eventhdlr eventhdlr): diff --git a/tests/test_event.py b/tests/test_event.py index 7b11980b9..87047315f 100644 --- a/tests/test_event.py +++ b/tests/test_event.py @@ -1,3 +1,7 @@ +import gc +import sys +import weakref + import pytest, random from pyscipopt import Model, Eventhdlr, SCIP_RESULT, SCIP_EVENTTYPE, SCIP_PARAMSETTING, quicksum @@ -189,4 +193,37 @@ def eventexec(self, event): m.includeEventhdlr(ev, "var_event", "event handler for var events") with pytest.raises(Exception): - m.optimize() \ No newline at end of file + m.optimize() + +def test_catchEvent_does_not_leak_model(): + """catchEvent should not artificially increment the Model's reference count. + + Previously, catchEvent called Py_INCREF(self) on the Model, and dropEvent + called Py_DECREF(self). Since many event handlers skip dropEvent (e.g. when + self.model is already dead), this caused the Model to leak — its refcount + never returned to zero, preventing garbage collection. + """ + + class SimpleEvent(Eventhdlr): + def eventinit(self): + self.model.catchEvent(SCIP_EVENTTYPE.NODEFOCUSED, self) + + def eventexit(self): + pass # intentionally no dropEvent, as is common in practice + + def eventexec(self, event): + pass + + m = Model() + m.hideOutput() + ev = SimpleEvent() + m.includeEventhdlr(ev, "simple", "test event handler") + m.addVar("x", obj=1, vtype="I") + m.optimize() + + ref = weakref.ref(m) + del ev + del m + gc.collect() + + assert ref() is None, "Model was not garbage collected — catchEvent likely leaked a reference" \ No newline at end of file From 8e76c1344a904ad4116e4e4515070e2cb78b5a12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Dion=C3=ADsio?= <57299939+Joao-Dionisio@users.noreply.github.com> Date: Thu, 19 Feb 2026 15:04:41 +0000 Subject: [PATCH 2/7] Apply suggestions from code review Co-authored-by: DominikKamp <130753997+DominikKamp@users.noreply.github.com> --- tests/test_event.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/test_event.py b/tests/test_event.py index 87047315f..02cb074ce 100644 --- a/tests/test_event.py +++ b/tests/test_event.py @@ -1,8 +1,4 @@ -import gc -import sys -import weakref - -import pytest, random +import pytest, weakref, gc, random from pyscipopt import Model, Eventhdlr, SCIP_RESULT, SCIP_EVENTTYPE, SCIP_PARAMSETTING, quicksum @@ -209,7 +205,7 @@ def eventinit(self): self.model.catchEvent(SCIP_EVENTTYPE.NODEFOCUSED, self) def eventexit(self): - pass # intentionally no dropEvent, as is common in practice + pass # intentionally no dropEvent, which is bad practice def eventexec(self, event): pass @@ -226,4 +222,10 @@ def eventexec(self, event): del m gc.collect() + del m + gc.collect() + assert ref() is not None, "Model was accidentally garbage collected — event handler active" + + del ev + gc.collect() assert ref() is None, "Model was not garbage collected — catchEvent likely leaked a reference" \ No newline at end of file From c36cb0d1a40267fe0d6d59bde82bbc27003c7223 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Dion=C3=ADsio?= <57299939+Joao-Dionisio@users.noreply.github.com> Date: Thu, 19 Feb 2026 15:18:11 +0000 Subject: [PATCH 3/7] Update tests/test_event.py Co-authored-by: DominikKamp <130753997+DominikKamp@users.noreply.github.com> --- tests/test_event.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/test_event.py b/tests/test_event.py index 02cb074ce..9da0ba7c3 100644 --- a/tests/test_event.py +++ b/tests/test_event.py @@ -218,9 +218,6 @@ def eventexec(self, event): m.optimize() ref = weakref.ref(m) - del ev - del m - gc.collect() del m gc.collect() From fda89d8ab43c32ac33e52a1a6ff28feae59c0812 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Dion=C3=ADsio?= <57299939+Joao-Dionisio@users.noreply.github.com> Date: Thu, 19 Feb 2026 21:02:18 +0000 Subject: [PATCH 4/7] Update tests/test_event.py Co-authored-by: DominikKamp <130753997+DominikKamp@users.noreply.github.com> --- tests/test_event.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_event.py b/tests/test_event.py index 9da0ba7c3..1b54209d0 100644 --- a/tests/test_event.py +++ b/tests/test_event.py @@ -219,10 +219,10 @@ def eventexec(self, event): ref = weakref.ref(m) - del m + del ev gc.collect() - assert ref() is not None, "Model was accidentally garbage collected — event handler active" + assert ref() is not None, "Model was garbage collected — event handler absorbed a reference" - del ev + del m gc.collect() assert ref() is None, "Model was not garbage collected — catchEvent likely leaked a reference" \ No newline at end of file From 12d901aa8caec080c77454da32c30dd2b287de1f Mon Sep 17 00:00:00 2001 From: Joao-Dionisio Date: Thu, 19 Feb 2026 22:57:41 +0000 Subject: [PATCH 5/7] Add changelog entries for unreleased changes --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e63edb73..7f62a8bc0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,9 @@ ## Unreleased ### Added +- Added `getMemUsed()`, `getMemTotal()`, and `getMemExternEstim()` methods ### Fixed +- Removed incorrect `Py_INCREF`/`Py_DECREF` on `Model` in `catchEvent`/`dropEvent` that caused reference count imbalance ### Changed ### Removed From b67b7b03942e74fbc374db20375a78b6a7d7c54d Mon Sep 17 00:00:00 2001 From: Joao-Dionisio Date: Fri, 20 Feb 2026 15:22:41 +0000 Subject: [PATCH 6/7] Fix incomplete docstring in test_catchEvent_does_not_leak_model --- tests/test_event.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test_event.py b/tests/test_event.py index 1b54209d0..629596126 100644 --- a/tests/test_event.py +++ b/tests/test_event.py @@ -195,9 +195,11 @@ def test_catchEvent_does_not_leak_model(): """catchEvent should not artificially increment the Model's reference count. Previously, catchEvent called Py_INCREF(self) on the Model, and dropEvent - called Py_DECREF(self). Since many event handlers skip dropEvent (e.g. when - self.model is already dead), this caused the Model to leak — its refcount - never returned to zero, preventing garbage collection. + called Py_DECREF(self). In practice these calls are often unbalanced — event + handlers commonly catch events without a matching drop (e.g. calling + catchEvent in eventinit but omitting dropEvent in eventexit). Each unmatched + catchEvent permanently inflated the Model's refcount, preventing garbage + collection. """ class SimpleEvent(Eventhdlr): From 063e0055d1a977d38f34dfd59df93a2bd63ff242 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Dion=C3=ADsio?= <57299939+Joao-Dionisio@users.noreply.github.com> Date: Fri, 20 Feb 2026 15:23:05 +0000 Subject: [PATCH 7/7] Update CHANGELOG.md Co-authored-by: DominikKamp <130753997+DominikKamp@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f62a8bc0..474b83b67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Added - Added `getMemUsed()`, `getMemTotal()`, and `getMemExternEstim()` methods ### Fixed -- Removed incorrect `Py_INCREF`/`Py_DECREF` on `Model` in `catchEvent`/`dropEvent` that caused reference count imbalance +- Removed `Py_INCREF`/`Py_DECREF` on `Model` in `catchEvent`/`dropEvent` that caused memory leak for imbalanced usage ### Changed ### Removed