Skip to content

Commit

Permalink
Fix reference counting leak in nrnpy_restore_savestate_.
Browse files Browse the repository at this point in the history
The facts:
  1. `PyByteArray_FromStringAndSize` returns a "new reference" [1].
  2. `PyObject_CallObject` returns a "new reference" [2].
  3. `PyTuple_SetItem` steals `py_data` [3].

Let's analyse `py_data` first:
  * After `PyByteArray_FromStringAndSize` it's at `+1` (meaning 1
    unaccounted INCREF).
  * After `Py_INCREF(py_data)` it's at `+2`.
  * By calling `PyTuple_SetItem` we know 1 DECREF will happen, because
    it steals `py_data`. The reference count of `py_data` is at `+1`.

Therefore, there's one INCREF for which we can't pair up with a DECREF.

Let's move on to `result`, since it's a "new reference" we're
responsible for calling `DECREF`.

[1]: https://docs.python.org/3/c-api/bytearray.html#c.PyByteArray_FromStringAndSize
[2]: https://docs.python.org/3/c-api/call.html#c.PyObject_CallObject
[3]: https://docs.python.org/3/c-api/tuple.html#c.PyTuple_SetItem
  • Loading branch information
1uc committed Nov 15, 2024
1 parent 36084d9 commit ffeee68
Showing 1 changed file with 5 additions and 6 deletions.
11 changes: 5 additions & 6 deletions src/nrnpython/nrnpy_hoc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2638,16 +2638,15 @@ static void nrnpy_store_savestate_(char** save_data, uint64_t* save_data_size) {
static void nrnpy_restore_savestate_(int64_t size, char* data) {
if (restore_savestate_) {
PyObject* args = PyTuple_New(1);
PyObject* py_data = PyByteArray_FromStringAndSize(data, size);
Py_INCREF(py_data);
if (py_data == NULL) {
auto py_data = nb::steal(PyByteArray_FromStringAndSize(data, size));
if (!py_data) {
hoc_execerror("SaveState:", "Data restore failure.");
}
// note: PyTuple_SetItem steals a ref to py_data
PyTuple_SetItem(args, 0, py_data);
PyObject* result = PyObject_CallObject(restore_savestate_, args);
PyTuple_SetItem(args, 0, py_data.release().ptr());
auto result = nb::steal(PyObject_CallObject(restore_savestate_, args));
Py_DECREF(args);
if (result == NULL) {
if (!result) {
hoc_execerror("SaveState:", "Data restore failure.");
}
} else {
Expand Down

0 comments on commit ffeee68

Please sign in to comment.