changeset 116:32a67788a336

fix memory leaks in orthanc.RestApiPost() and sibling methods
author Alain Mazy <am@osimis.io>
date Mon, 28 Aug 2023 13:08:44 +0200
parents 717f15a5b9a6
children 1029689d7b6e
files NEWS Sources/RestCallbacks.cpp
diffstat 2 files changed, 34 insertions(+), 25 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Mon Aug 14 10:18:41 2023 +0200
+++ b/NEWS	Mon Aug 28 13:08:44 2023 +0200
@@ -1,6 +1,8 @@
 Pending changes in the mainline
 ===============================
 
+Maintenance:
+* Fix memory leaks when a python script calls orthanc.RestApiPost() and sibling methods
 * New builders for Windows: Supporting 32 / 64bit with Python 3.9 / 3.10 / 3.11
 
 
--- a/Sources/RestCallbacks.cpp	Mon Aug 14 10:18:41 2023 +0200
+++ b/Sources/RestCallbacks.cpp	Mon Aug 28 13:08:44 2023 +0200
@@ -125,53 +125,60 @@
 
       PythonObject kw(lock, PyDict_New());
 
+      // We must handle memory ourselves for all members of kw that are set through PyDict_SetItem or PyDict_SetItemString.
+      // The Python documentation states that these methods do not steal references to the objects.  
+      // We have indeed observed memory leaks when not handling memory this way.
+      std::unique_ptr<PythonObject> pyGet;
+      std::vector<boost::shared_ptr<PythonString> > pyGetValues;
+      std::unique_ptr<PythonObject> pyHeaders;
+      std::vector<boost::shared_ptr<PythonString> > pyHeaderValues;
+      std::unique_ptr<PythonObject> pyBody;
+      std::unique_ptr<PythonObject> pyGroups;
+
+      PythonString pyMethod(lock, method);
+      PyDict_SetItemString(kw.GetPyObject(), "method", pyMethod.GetPyObject());
+
+      PythonObject pyGroups(lock, PyTuple_New(request->groupsCount));
+      for (uint32_t i = 0; i < request->groupsCount; i++)
       {
-        PythonString tmp(lock, method);
-        PyDict_SetItemString(kw.GetPyObject(), "method", tmp.Release());
+        PythonString tmp(lock, request->groups[i]);
+        PyTuple_SetItem(pyGroups->GetPyObject(), i, tmp.Release()); // this PyTuple_SetItem method "steals" a reference -> no need to manage memory by ourselves !
       }
 
-      {
-        PythonObject groups(lock, PyTuple_New(request->groupsCount));
-
-        for (uint32_t i = 0; i < request->groupsCount; i++)
-        {
-          PythonString tmp(lock, request->groups[i]);
-          PyTuple_SetItem(groups.GetPyObject(), i, tmp.Release());
-        }
-
-        PyDict_SetItemString(kw.GetPyObject(), "groups", groups.Release());
-      }
+      PyDict_SetItemString(kw.GetPyObject(), "groups", pyGroups->GetPyObject());
 
       if (request->method == OrthancPluginHttpMethod_Get)
       {
-        PythonObject get(lock, PyDict_New());
-
+        pyGet.reset(new PythonObject(lock, PyDict_New()));
         for (uint32_t i = 0; i < request->getCount; i++)
         {
-          PythonString value(lock, request->getValues[i]);
-          PyDict_SetItemString(get.GetPyObject(), request->getKeys[i], value.Release());
+          boost::shared_ptr<PythonString> value(new PythonString(lock, request->getValues[i]));
+          PyDict_SetItemString(pyGet->GetPyObject(), request->getKeys[i], value->GetPyObject());
+          pyGetValues.push_back(value);
         }
 
-        PyDict_SetItemString(kw.GetPyObject(), "get", get.Release());
+        PyDict_SetItemString(kw.GetPyObject(), "get", pyGet->GetPyObject());
       }
 
       {
-        PythonObject headers(lock, PyDict_New());
-
+        pyHeaders.reset(new PythonObject(lock, PyDict_New()));
         for (uint32_t i = 0; i < request->headersCount; i++)
         {
-          PythonString value(lock, request->headersValues[i]);
-          PyDict_SetItemString(headers.GetPyObject(), request->headersKeys[i], value.Release());
+          boost::shared_ptr<PythonString> value(new PythonString(lock, request->headersValues[i]));
+          PyDict_SetItemString(pyHeaders->GetPyObject(), request->headersKeys[i], value->GetPyObject());
+          pyHeaderValues.push_back(value);
         }
 
-        PyDict_SetItemString(kw.GetPyObject(), "headers", headers.Release());
+        PyDict_SetItemString(kw.GetPyObject(), "headers", pyHeaders->GetPyObject());
       }
 
       if (request->method == OrthancPluginHttpMethod_Post ||
           request->method == OrthancPluginHttpMethod_Put)
       {
-        PyDict_SetItemString(kw.GetPyObject(), "body", PyBytes_FromStringAndSize(
-                               reinterpret_cast<const char*>(request->body), request->bodySize));
+        pyBody.reset(new PythonObject(lock, PyBytes_FromStringAndSize(
+                                      reinterpret_cast<const char*>(request->body), request->bodySize)));
+
+        PyDict_SetItemString(kw.GetPyObject(), "body", pyBody->GetPyObject());
       }
 
       /**