gen_do_trig in trighooks.c

[melzaren]

If you look here you'll notice... `` # // if triggers create methods, it increases the reference count on our // dictionary. This is a known memory leak. We'll try to mitigate it by // cleaning out some of the contents. A better solution is needed, that // makes sure the dictionary itself is not leaked LIST_ITERATOR vname_i = newListIterator(varnames); char vname = NULL; ITERATE_LIST(vname, vname_i) { PyDict_DelItemString(dict, vname); } deleteListIterator(vname_i); deleteListWith(varnames, free);

// what about PyDict_Clear?

Py_XDECREF(dict); ``

I added the // at the end there. It seems in later versions of CPython PyDict_Clear was added. Perhaps it can be used to deal with the methods created from within the script in order to deal with the leak?

[melzaren]

Maybe we test the clearing and do a reference count check before and after to see what's going on.

https://docs.python.org/3/c-api/refcounting.html#c.Py_REFCNT

[melzaren]

I changed gen_do_trig to this (at the bottom of the function):

log_string("Dictionary reference count: %ld", Py_REFCNT(dict));

// if triggers create methods, it increases the reference count on our // dictionary. This is a known memory leak. We'll try to mitigate it by // cleaning out some of the contents. A better solution is needed, that // makes sure the dictionary itself is not leaked LIST_ITERATOR vname_i = newListIterator(varnames); char vname = NULL; ITERATE_LIST(vname, vname_i) { PyDict_DelItemString(dict, vname); } deleteListIterator(vname_i); deleteListWith(varnames, free);

log_string("Dictionary reference count: %ld", Py_REFCNT(dict));

PyDict_Clear(dict); log_string("Dictionary reference count: %ld", Py_REFCNT(dict));

Py_XDECREF(dict);

You can see that PyDict_Clear does indeed leave the dictionary with only one reference remaining before the final call to Py_XDECREF, so I /think/ this works.

I actually think we can get rid of the loop above PyDict_Clear(dict) which is now redundant.

As always I encourage you to vet these changes and test them to make sure they're correct/stable.

[LimpingNinja]

Thanks for doing the testing here, you are correct in that PyDict_Clear is the right path; and that we can get rid of the redundant loop - it isn't needed any longer and PyDict_Clear is the safer choice.

  // run the script, then kill our dictionary
  triggerRun(trig, dict);

  PyDict_Clear(dict);
  deleteListWith(varnames, free);
  Py_XDECREF(dict);

As a side note you can reference the code lines in GitHub, by browsing to the file and clicking the line number to copy the permalink. This references: https://github.com/LimpingNinja/nakedmud/blob/dd5ec10bf7721d4caa5a32d9cac4c69d18c47bef/src/scripts/trighooks.c#L161-L166

[melzaren]

The solution you implemented removes deleteListWith, so the varnames list never gets cleaned up, so there's still a memory leak.

It seems to me that the list itself is no longer necessary since its entire raison d'etre was to serve as a tool for cleaning out the dictionary.