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
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()
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
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
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