Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:1318
Return-Path: <msisolak@yahoo.com>
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Delivered-To: mailing list internals@lists.php.net
Received: (qmail 78050 invoked from network); 6 May 2003 15:52:23 -0000
Received: from unknown (HELO web13409.mail.yahoo.com) (216.136.172.17)
  by pb1.pair.com with SMTP; 6 May 2003 15:52:23 -0000
Message-ID: <20030506155222.30298.qmail@web13409.mail.yahoo.com>
Received: from [66.114.66.34] by web13409.mail.yahoo.com via HTTP; Tue, 06 May 2003 08:52:22 PDT
Date: Tue, 6 May 2003 08:52:22 -0700 (PDT)
To: internals@lists.php.net
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="0-648367161-1052236342=:29432"
Subject: [PATCH 4.3.2RC2] COM Property Put Memory Leak
From: msisolak@yahoo.com (Michael Sisolak)

--0-648367161-1052236342=:29432
Content-Type: text/plain; charset=us-ascii
Content-Id: 
Content-Disposition: inline

Attached is a patch for a memory leak that occurs when setting a COM
property to a string value.  According to Microsoft, when passing
parameters to Invoke
(http://msdn.microsoft.com/library/en-us/automat/htm/chap5_3kz7.asp):

"The calling code is responsible for releasing all strings and objects
referred to by rgvarg[ ] [the parameters passed to the Invoke call] or
placed in *pVarResult. As with other parameters that are passed by
value, if the invoked member must maintain access to a string after
returning, you should copy the string."

An example of this can be seen in the following code from MSDN (second
code block of
http://msdn.microsoft.com/library/en-us/automat/htm/chap3_76p4.asp):

   hr = pdisp->Invoke(dispid, IID_NULL, LOCALE_USER_DEFAULT, wFlags,
        &dispparams, pvRet, pexcepinfo, pnArgErr);

cleanup:
    // Clean up any arguments that need it.
    if (dispparams.cArgs != 0)
    {
        VARIANTARG * pvarg = dispparams.rgvarg;
        UINT cArgs = dispparams.cArgs;
        while (cArgs--)
        {
            switch (pvarg->vt)
            {
            case VT_BSTR:
                VariantClear(pvarg);
                break;
            }
            ++pvarg;
        }
    }
    delete dispparams.rgvarg;

The 4.3.2RC2 version of do_COM_propput() in ext/COM/COM.c, however,
only efrees the parameter array without first checking if the property
was a string that needs to be freed by the caller.  The attached patch
checks for a VT_BSTR parameter and calls VariantClear() if approprate.

Without this patch the following HTML/PHP page will grow a 4.3.2RC2
(under IIS5/PHP4ISAPI) processes memory usage without bound:

<?php

$xml = new COM("MSXML.DOMDocument");
$element = $xml->createElement("test");
$element->text = str_repeat("this is a text string!", 1000);
$val_a = com_get($element, "text");
$element->Release();
$xml->Release();

echo "val_a = {$val_a}<br><br>";
?>

<script>
setTimeout("document.location.reload(true);", 100);
</script>

With this patch in place there are no memory leaks when this code is
left to run.

Michael Sisolak
msisolak@yahoo.com

__________________________________
Do you Yahoo!?
The New Yahoo! Search - Faster. Easier. Bingo.
http://search.yahoo.com
--0-648367161-1052236342=:29432
Content-Type: text/plain; name="COM.c.patch"
Content-Description: COM.c.patch
Content-Disposition: inline; filename="COM.c.patch"

--- COM.c.orig	Tue May 06 08:56:38 2003
+++ COM.c	Tue May 06 09:00:32 2003
@@ -1627,6 +1627,11 @@
 
 		FREE_VARIANT(var_result);
 
+		/* free caller created strings (invoked must make it's own copy) */
+		if (V_VT(new_value) == VT_BSTR) {
+			VariantClear(new_value);
+		}
+
 		efree(new_value);
 		efree(propname);
 
@@ -1659,7 +1664,12 @@
 		FREE_VARIANT(var_result);
 	}
 
-	efree(new_value); // FREE_VARIANT does a VariantClear() which is not desired here !
+	/* free caller created strings (invoked must make it's own copy) */
+	if (V_VT(new_value) == VT_BSTR) {
+		VariantClear(new_value);
+	}
+
+	efree(new_value);
 	efree(propname);
 }
 

--0-648367161-1052236342=:29432--