Memory Leak in Safearray Conversion

[ghost]

In the method PyCom_PyObjectFromSAFEARRAYDimensionItem elements are retrieved via the SafeArrayGetElement method. The SafeArrayGetElement method always returns a copy of the internal data. In case of BSTR, IDispatch and IUnkown these copies are not freed or attached to a Python object. This patch attaches the returned values to the corresponding Python objects.

Reported by: sschukat

Original Ticket: pywin32/bugs/640

[ghost]

Patch fixing memory leak.

Original comment by: sschukat

[Avasam]

Tagging @sschukat as the original SourceForge issue creator.

Proposed patch with updated formatting and location:

diff --git a/com/win32com/src/oleargs.cpp b/com/win32com/src/oleargs.cpp
index c6fad6c2..b6d7abe4 100644
--- a/com/win32com/src/oleargs.cpp
+++ b/com/win32com/src/oleargs.cpp
@@ -860,7 +860,7 @@ static PyObject *PyCom_PyObjectFromSAFEARRAYDimensionItem(SAFEARRAY *psa, VARENU
             hres = SafeArrayGetElement(psa, arrayIndices, &str);
             if (FAILED(hres))
                 break;
-            subitem = PyWinObject_FromBstr(str);
+            subitem = PyWinObject_FromBstr(str, TRUE);
             break;
         }
         case VT_DISPATCH: {
@@ -868,7 +868,7 @@ static PyObject *PyCom_PyObjectFromSAFEARRAYDimensionItem(SAFEARRAY *psa, VARENU
             hres = SafeArrayGetElement(psa, arrayIndices, &pDisp);
             if (FAILED(hres))
                 break;
-            subitem = PyCom_PyObjectFromIUnknown(pDisp, IID_IDispatch, TRUE);
+            subitem = PyCom_PyObjectFromIUnknown(pDisp, IID_IDispatch, FALSE);
             break;
         }
         // case VT_ERROR - handled above with I4
@@ -895,7 +895,7 @@ static PyObject *PyCom_PyObjectFromSAFEARRAYDimensionItem(SAFEARRAY *psa, VARENU
             hres = SafeArrayGetElement(psa, arrayIndices, &pUnk);
             if (FAILED(hres))
                 break;
-            subitem = PyCom_PyObjectFromIUnknown(pUnk, IID_IUnknown, TRUE);
+            subitem = PyCom_PyObjectFromIUnknown(pUnk, IID_IUnknown, FALSE);
             break;
         }
             // case VT_DECIMAL

May relate to #1864

[kjpowerworld]

Would it be worth me adding a PR to address the issue utilizing the patch you've added @Avasam? I've reviewed the patch that's here (originally in the SF bug ticket) and I concur with the fix. This would be valuable to get into pywin32 sooner rather than later as it does impact some customers of the company that I work at.

[Avasam]

@kjpowerworld Feel free to do so! It's not my area of expertise but it looks simple enough, Mark should be able to review.

I've also been meaning to find all the patches migrated to GitHub and create proper PRs from them (even if I don't necessarily understand the implication of the changes) to make them easier to review and/or deny.

[sschukat]

If you have any question regarding the fix feel free to ask. But as stated it is a simple fix, since the ownership given by SafeArrayGetElement is transferred to the corresponding Python objects.