PHP extension - Saxon/C

  100749
September 22, 2017 11:01 oneil@saxonica.com (O'Neil Delpratt)
Hi,

I am having a problem in my PHP extension and I am wondering if you can give me some advise or even a pointer. 

We have the following class object XQueryProcessor and XdmNode.

The following method takes the XdmNode as parameter: XQueryProcessor.setContextItem(XdmNode)

Any idea what is going wrong in the code below? The XdmNode in my PHP class is a wrapper for a C/C++ object which is held in the XQueryProcessor. Do I have to add some special refcount to the PHP XdmNode?

This is giving a memory leak warning:

[Fri Sep 22 08:56:42 2017]  Script:  '/home/ond1/work/svn/latest9.8-saxonc/hec/samples/php/xqueryExamples.php'
/home/ond1/work/svn/latest9.8-saxonc/hec/Saxon.C.API/php7_saxon.cpp(3250) :  Freeing 0x00007ffff68863c0 (48 bytes), script=/home/ond1/work/svn/latest9.8-saxonc/hec/samples/php/xqueryExamples.php
/home/ond1/work/svn/latest9.8-saxonc/hec/Saxon.C.API/php7-src/php-src/Zend/zend_alloc.c(2472) : Actual location (location was relayed)
=== Total 1 memory leaks detected ===


The code in question is at follows:

PHP_METHOD(XQueryProcessor, setContextItem)
{
   char * context;
   int len1;
   zval* oth;
    XQueryProcessor *xqueryProcessor;

    if (zend_parse_parameters(ZEND_NUM_ARGS(), "z", &oth) == FAILURE) {
        RETURN_NULL();
    }
    if(oth != NULL) {
    	zend_object* pobj = Z_OBJ_P(getThis()); 
    	xqueryProcessor_object *obj = (xqueryProcessor_object *)((char *)pobj - XtOffsetOf(xqueryProcessor_object, std));
    	xqueryProcessor = obj->xqueryProcessor;
    const char * objName =ZSTR_VAL(Z_OBJCE_P(oth)->name);
Z_ADDREF_P(oth);
	
      if(strcmp(objName, "Saxon\\XdmNode")==0) {
	zend_object *vvobj =  Z_OBJ_P(oth);
	xdmNode_object* ooth  = (xdmNode_object *)((char *)vvobj - XtOffsetOf(xdmNode_object, std));
        if(ooth != NULL) {
            XdmNode * value = ooth->xdmNode;
            if(value != NULL) {
Z_ADDREF(*oth);	
	        xqueryProcessor->setContextItem((XdmItem *)value);
	        return;
            }
        }
      } else
…….

See full code of PHP extension:

https://dev.saxonica.com/repos/archive/opensource/latest9.8/hec/Saxon.C.API/php7_saxon.cpp <https://dev.saxonica.com/repos/archive/opensource/latest9.8/hec/Saxon.C.API/php7_saxon.cpp>


kind regards,

O'Neil


-------------------------------
O'Neil Delpratt
Software Developer, Saxonica Limited 
Email: oneil@saxonica.com
Twitter: https://twitter.com/ond1
Tel: +44 118 946 5894
Web: http://www.saxonica.com
Saxonica Community site: http://dev.saxonica.com
Bug tracking site: https://saxonica.plan.io/
  100751
September 22, 2017 14:40 johannes@schlueters.de (Johannes =?ISO-8859-1?Q?Schl=FCter?=)
On Fr, 2017-09-22 at 12:01 +0100, O'Neil Delpratt wrote:
>  > [Fri Sep 22 08:56:42 2017]  Script:  '/home/ond1/work/svn/latest9.8- > saxonc/hec/samples/php/xqueryExamples.php' > /home/ond1/work/svn/latest9.8- > saxonc/hec/Saxon.C.API/php7_saxon.cpp(3250) :  Freeing  [...]
> See full code of PHP extension: > > https://dev.saxonica.com/repos/archive/opensource/latest9.8/hec/Saxon > .C.API/php7_saxon.cpp > <https://dev.saxonica.com/repos/archive/opensource/latest9.8/hec/Saxo > n.C.API/php7_saxon.cpp>
could you point out which line  3250 is? In the linked file line 3250 refers to this line: PHP_METHOD(XdmNode, __construct) {    /* <------------------------------------ 3250 */ php_error(E_WARNING,"XdmNode constructor");     XdmNode *xdmNode = NULL; which makes little sense :-) johannes
  100752
September 22, 2017 15:06 oneil@saxonica.com (O'Neil Delpratt)
> On 22 Sep 2017, at 15:40, Johannes Schlüter <johannes@schlueters.de> wrote: > > On Fr, 2017-09-22 at 12:01 +0100, O'Neil Delpratt wrote: >> >> [Fri Sep 22 08:56:42 2017] Script: '/home/ond1/work/svn/latest9.8- >> saxonc/hec/samples/php/xqueryExamples.php' >> /home/ond1/work/svn/latest9.8- >> saxonc/hec/Saxon.C.API/php7_saxon.cpp(3250) : Freeing > [...] >> See full code of PHP extension: >> >> https://dev.saxonica.com/repos/archive/opensource/latest9.8/hec/Saxon >> .C.API/php7_saxon.cpp >> <https://dev.saxonica.com/repos/archive/opensource/latest9.8/hec/Saxo >> n.C.API/php7_saxon.cpp> > > could you point out which line 3250 is? In the linked file line 3250 > refers to this line:
I think this is now 3243: xdmNode_object *obj = (xdmNode_object *)ecalloc(1, sizeof(xdmNode_object)+ zend_object_properties_size(type)); Which seems strange to me.
> > > > PHP_METHOD(XdmNode, __construct) > { /* <------------------------------------ 3250 */ > php_error(E_WARNING,"XdmNode constructor"); > XdmNode *xdmNode = NULL; > > > which makes little sense :-) >
Yes indeed. Some left over code which is not doing anything because a XdmNode can only be created by a utility method on other classes.
> johannes >
  100753
September 22, 2017 16:09 pollita@php.net (Sara Golemon)
On Fri, Sep 22, 2017 at 7:01 AM, O'Neil Delpratt <oneil@saxonica.com> wrote:
> Z_ADDREF_P(oth); > It's a little rough staring through all the commented out lines and
inconsistent indenting, but this line stands out to me. Where is oth's refcount meant to be decremented? Why is it even being incremented in the first place? By having an extra reference, the engine doesn't know that it's supposed to clean up the object and free its storage. -Sara
  100754
September 22, 2017 16:21 oneil@saxonica.com (O'Neil Delpratt)
I still have the memory leak without that line. I think I was thinking I need to add a refcount to the XdmNode object, but maybe I am wrong.


> On 22 Sep 2017, at 17:09, Sara Golemon <pollita@php.net> wrote: > > On Fri, Sep 22, 2017 at 7:01 AM, O'Neil Delpratt <oneil@saxonica.com> wrote: >> Z_ADDREF_P(oth); >> > It's a little rough staring through all the commented out lines and > inconsistent indenting, but this line stands out to me. > > Where is oth's refcount meant to be decremented? Why is it even being > incremented in the first place? By having an extra reference, the > engine doesn't know that it's supposed to clean up the object and free > its storage. > > -Sara
  100755
September 22, 2017 16:32 pollita@php.net (Sara Golemon)
On Fri, Sep 22, 2017 at 12:21 PM, O'Neil Delpratt <oneil@saxonica.com> wrote:
> I still have the memory leak without that line. I think I was thinking I need to add a refcount to the XdmNode object, but maybe I am wrong. > What's the shortest possible script you can reproduce the leak with?
Is (new XQueryProcessor()) enough? Do you need to call particular methods (like this one) with certain args?
  100756
September 22, 2017 16:44 oneil@saxonica.com (O'Neil Delpratt)
The Following gives leak:

$proc = Saxon/SaxonProcessor

$xquery = $proc->NewXQueryProcessor()

$sourceNode = $proc->parseXmlFromString("”); 

$xquery->setContextItem($sourceNode); // with this line commented out there is no leak.


> On 22 Sep 2017, at 17:32, Sara Golemon <pollita@php.net> wrote: > > On Fri, Sep 22, 2017 at 12:21 PM, O'Neil Delpratt <oneil@saxonica.com> wrote: >> I still have the memory leak without that line. I think I was thinking I need to add a refcount to the XdmNode object, but maybe I am wrong. >> > What's the shortest possible script you can reproduce the leak with? > > Is (new XQueryProcessor()) enough? Do you need to call particular > methods (like this one) with certain args?
  100757
September 22, 2017 17:42 pollita@php.net (Sara Golemon)
>> On Fri, Sep 22, 2017 at 12:21 PM, O'Neil Delpratt <oneil@saxonica.com> wrote: >> I still have the memory leak without that line. I think I was thinking I need to add a refcount to the XdmNode object, but maybe I am wrong. >> > The Following gives leak: > > $proc = Saxon/SaxonProcessor > > $xquery = $proc->NewXQueryProcessor() > > $sourceNode = $proc->parseXmlFromString("”); > > $xquery->setContextItem($sourceNode); // with this line commented out there is no leak. > Okay, well the bad news is that assuming the extension you're working
with is the link you gave initially and you did remove the Z_ADDREF_P() from XQueryProcessor::setContextItem()... I can't point to why you'd have a memory leak. -Sara
  100769
September 25, 2017 10:36 oneil@saxonica.com (O'Neil Delpratt)
Ok. Thanks for taking a look.

Potenial problem could be the following line:

const char * objName =ZSTR_VAL(Z_OBJCE_P(oth)->name);


I think I need to free the objName some how.


> On 22 Sep 2017, at 18:42, Sara Golemon <pollita@php.net> wrote: > >>> On Fri, Sep 22, 2017 at 12:21 PM, O'Neil Delpratt <oneil@saxonica.com> wrote: >>> I still have the memory leak without that line. I think I was thinking I need to add a refcount to the XdmNode object, but maybe I am wrong. >>> >> The Following gives leak: >> >> $proc = Saxon/SaxonProcessor >> >> $xquery = $proc->NewXQueryProcessor() >> >> $sourceNode = $proc->parseXmlFromString("”); >> >> $xquery->setContextItem($sourceNode); // with this line commented out there is no leak. >> > Okay, well the bad news is that assuming the extension you're working > with is the link you gave initially and you did remove the > Z_ADDREF_P() from XQueryProcessor::setContextItem()... I can't point > to why you'd have a memory leak. > > -Sara
  100770
September 25, 2017 10:46 oneil@saxonica.com (O'Neil Delpratt)
Problem resolved. It was actually the following line I had still there: Z_ADDREF_P(oth);
> On 25 Sep 2017, at 11:36, O'Neil Delpratt <oneil@saxonica.com> wrote: > > Ok. Thanks for taking a look. > > Potenial problem could be the following line: > > const char * objName =ZSTR_VAL(Z_OBJCE_P(oth)->name); > > > I think I need to free the objName some how. > > >> On 22 Sep 2017, at 18:42, Sara Golemon <pollita@php.net> wrote: >> >>>> On Fri, Sep 22, 2017 at 12:21 PM, O'Neil Delpratt <oneil@saxonica.com> wrote: >>>> I still have the memory leak without that line. I think I was thinking I need to add a refcount to the XdmNode object, but maybe I am wrong. >>>> >>> The Following gives leak: >>> >>> $proc = Saxon/SaxonProcessor >>> >>> $xquery = $proc->NewXQueryProcessor() >>> >>> $sourceNode = $proc->parseXmlFromString("”); >>> >>> $xquery->setContextItem($sourceNode); // with this line commented out there is no leak. >>> >> Okay, well the bad news is that assuming the extension you're working >> with is the link you gave initially and you did remove the >> Z_ADDREF_P() from XQueryProcessor::setContextItem()... I can't point >> to why you'd have a memory leak. >> >> -Sara > > > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php >