Welcome! Log In Create A New Profile

Advanced

[PHP-DEV] PHP extension - Saxon/C

Posted by O'Neil Delpratt 
O'Neil Delpratt
[PHP-DEV] PHP extension - Saxon/C
September 22, 2017 01:10PM
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 Schlüter
Re: [PHP-DEV] PHP extension - Saxon/C
September 22, 2017 04:50PM
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


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
O'Neil Delpratt
Re: [PHP-DEV] PHP extension - Saxon/C
September 22, 2017 05:10PM
> On 22 Sep 2017, at 15:40, Johannes Schlüter <[email protected]> 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
>



--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sara Golemon
Re: [PHP-DEV] PHP extension - Saxon/C
September 22, 2017 06:20PM
On Fri, Sep 22, 2017 at 7:01 AM, O'Neil Delpratt <[email protected]> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
O'Neil Delpratt
Re: [PHP-DEV] PHP extension - Saxon/C
September 22, 2017 06:30PM
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 <[email protected]> wrote:
>
> On Fri, Sep 22, 2017 at 7:01 AM, O'Neil Delpratt <[email protected]> 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



--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sara Golemon
Re: [PHP-DEV] PHP extension - Saxon/C
September 22, 2017 06:40PM
On Fri, Sep 22, 2017 at 12:21 PM, O'Neil Delpratt <[email protected]> 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?

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
O'Neil Delpratt
Re: [PHP-DEV] PHP extension - Saxon/C
September 22, 2017 06:50PM
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.


> On 22 Sep 2017, at 17:32, Sara Golemon <[email protected]> wrote:
>
> On Fri, Sep 22, 2017 at 12:21 PM, O'Neil Delpratt <[email protected]> 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?



--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sara Golemon
Re: [PHP-DEV] PHP extension - Saxon/C
September 22, 2017 07:50PM
>> On Fri, Sep 22, 2017 at 12:21 PM, O'Neil Delpratt <[email protected]> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
O'Neil Delpratt
Re: [PHP-DEV] PHP extension - Saxon/C
September 25, 2017 12:40PM
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 <[email protected]> wrote:
>
>>> On Fri, Sep 22, 2017 at 12:21 PM, O'Neil Delpratt <[email protected]> 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



--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
O'Neil Delpratt
Re: [PHP-DEV] PHP extension - Saxon/C
September 25, 2017 12:50PM
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 <[email protected]> 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 <[email protected]> wrote:
>>
>>>> On Fri, Sep 22, 2017 at 12:21 PM, O'Neil Delpratt <[email protected]> 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
>
>
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>



--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sorry, only registered users may post in this forum.

Click here to login