Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

## Unreleased
### Added
- Added `getMemUsed()`, `getMemTotal()`, and `getMemExternEstim()` methods
### Fixed
- Removed `Py_INCREF`/`Py_DECREF` on `Model` in `catchEvent`/`dropEvent` that caused memory leak for imbalanced usage
### Changed
### Removed

Expand Down
2 changes: 0 additions & 2 deletions src/pyscipopt/scip.pxi
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@
if rc == SCIP_OKAY:
pass
elif rc == SCIP_ERROR:
raise Exception('SCIP: unspecified error!')

Check failure on line 319 in src/pyscipopt/scip.pxi

View workflow job for this annotation

GitHub Actions / test-coverage (3.11)

SCIP: unspecified error!
elif rc == SCIP_NOMEMORY:
raise MemoryError('SCIP: insufficient memory error!')
elif rc == SCIP_READERROR:
Expand All @@ -335,7 +335,7 @@
raise Exception('SCIP: method cannot be called at this time'
+ ' in solution process!')
elif rc == SCIP_INVALIDDATA:
raise Exception('SCIP: error in input data!')

Check failure on line 338 in src/pyscipopt/scip.pxi

View workflow job for this annotation

GitHub Actions / test-coverage (3.11)

SCIP: error in input data!
elif rc == SCIP_INVALIDRESULT:
raise Exception('SCIP: method returned an invalid result code!')
elif rc == SCIP_PLUGINNOTFOUND:
Expand Down Expand Up @@ -11268,7 +11268,6 @@
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):
Expand All @@ -11288,7 +11287,6 @@
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):
Expand Down
42 changes: 40 additions & 2 deletions tests/test_event.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import pytest, random
import pytest, weakref, gc, random

from pyscipopt import Model, Eventhdlr, SCIP_RESULT, SCIP_EVENTTYPE, SCIP_PARAMSETTING, quicksum

Expand Down Expand Up @@ -189,4 +189,42 @@ def eventexec(self, event):
m.includeEventhdlr(ev, "var_event", "event handler for var events")

with pytest.raises(Exception):
m.optimize()
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). 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):
def eventinit(self):
self.model.catchEvent(SCIP_EVENTTYPE.NODEFOCUSED, self)

def eventexit(self):
pass # intentionally no dropEvent, which is bad 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
gc.collect()
assert ref() is not None, "Model was garbage collected — event handler absorbed a reference"

del m
gc.collect()
assert ref() is None, "Model was not garbage collected — catchEvent likely leaked a reference"