Everything

php

leaks related with virtual_file_ex()

php.internals

Nuno Lopes

19 years ago

Hello,

There are some leaks in PHP associated with virtual_file_ex() usage.
The problem is that that function doesn't free the new 'cwd' on error (path
not found).

Taking a look at http://lxr.php.net/ident?i=virtual_file_ex we can see the
function usage. For example, expand_filepath() free()s the cwd on failure,
while ZEND_INCLUDE_OR_EVAL_SPEC_CONST_HANDLER() doesn't.


I'm not sure if you want virtual_file_ex() to clean the cwd itself, or leave
that responsibility to the caller, but we should define the convention and
patch the relevant places (and fix those mem leaks).

One test that triggers the problem is: tests/lang/bug35176.phpt


Nuno

Nuno Lopes

19 years ago

OK, first patch to fix the 'tests/lang/bug35176.phpt' specific problem:
http://mega.ist.utl.pt/~ncpl/zend_include_memleak.txt
There might be other issues with virtual_file_ex() that I haven't fully
tested.

Nuno


----- Original Message -----
From: "Nuno Lopes" <nlopess@php.net>
To: "PHPdev" <internals@lists.php.net>
Sent: Saturday, September 16, 2006 12:35 PM
Subject: [PHP-DEV] leaks related with virtual_file_ex()

Ilia A.

19 years ago

On 17-Sep-06, at 8:03 AM, Nuno Lopes wrote:

> OK, first patch to fix the 'tests/lang/bug35176.phpt' specific
> problem: http://mega.ist.utl.pt/~ncpl/zend_include_memleak.txt
> There might be other issues with virtual_file_ex() that I haven't
> fully tested.

Can you not simply add the free before the zend_message_dispatcher
(ZMSG_FAILED_REQUIRE_FOPEN, Z_STRVAL_P(inc_filename)); line which is
followed by a fatal error and in all other instances rely on the
existing free() ?

Ilia Alshanetsky

Nuno Lopes

19 years ago

> On 17-Sep-06, at 8:03 AM, Nuno Lopes wrote:
>
>> OK, first patch to fix the 'tests/lang/bug35176.phpt' specific  problem:
>> http://mega.ist.utl.pt/~ncpl/zend_include_memleak.txt
>> There might be other issues with virtual_file_ex() that I haven't  fully
>> tested.
>
> Can you not simply add the free before the zend_message_dispatcher
> (ZMSG_FAILED_REQUIRE_FOPEN, Z_STRVAL_P(inc_filename)); line which is
> followed by a fatal error and in all other instances rely on the  existing
> free() ?
>
> Ilia Alshanetsky

well it depends of the zend api.. If you consider that that error message
will be fatal and the include one won't be, well, your solution is simpler,
otherwise it needs the bigger patch. Anyway, I agree with you (whoever uses
zend engine without PHP will have to have that in attention).

Nuno