Everything

php

PHP extension - Saxon/C

php.internals

O'Neil Delpratt

8 years ago

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/

Johannes Schlueter

8 years ago

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

O'Neil Delpratt

8 years ago

> 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.

Sara Golemon

8 years ago

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

O'Neil Delpratt

8 years ago

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.

Sara Golemon

8 years ago

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?

O'Neil Delpratt

8 years ago

The Following gives leak:

$proc = Saxon/SaxonProcessor

$xquery = $proc->NewXQueryProcessor()

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

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

Sara Golemon

8 years ago

>> 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("<foo />”);
>
> $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

O'Neil Delpratt

8 years ago

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.

O'Neil Delpratt

8 years ago

Problem resolved. It was actually the following line I had still there: Z_ADDREF_P(oth);