laruence / php-lua

This extension embeds the lua interpreter and offers an OO-API to lua variables and functions.
http://pecl.php.net/package/lua
Other
149 stars 50 forks source link

Memleaks #16

Closed hjanuschka closed 9 years ago

hjanuschka commented 9 years ago

no more outstanding memory leaks

ALL tests pass with -m

make test TEST_PHP_ARGS="-m --show-diff -m"
=====================================================================
PHP         : /usr/local/bin/php 
PHP_SAPI    : cli
PHP_VERSION : 7.0.0-dev
ZEND_VERSION: 3.0.0-dev
PHP_OS      : Linux - Linux ip-172-31-14-228 3.2.0-4-amd64 #1 SMP Debian 3.2.63-2 x86_64
INI actual  : /home/admin/tmp/php-lua/tmp-php.ini
More .INIs  :  
CWD         : /home/admin/tmp/php-lua
Extra dirs  : 
VALGRIND    : valgrind-3.7.0
=====================================================================
TIME START 2015-02-04 10:07:01
=====================================================================
PASS Basic lua check [tests/001.phpt] 
PASS Check for Table Pass vic-a-verse [tests/0014.phpt] 
PASS Set and read properties [tests/002.phpt] 
PASS Call lua functions [tests/003.phpt] 
PASS Type conversion from lua to PHP [tests/004.phpt] 
PASS Lua phpinfo() block [tests/005.phpt] 
PASS Lua::include() [tests/006.phpt] 
PASS Lua return function [tests/007.phpt] 
PASS register php function to lua [tests/008.phpt] 
PASS Bug (eval and include compute wrong return value number) [tests/009.phpt] 
PASS LuaClosure exception [tests/010.phpt] 
PASS register invalid php callback to lua [tests/011.phpt] 
PASS Lua::include() with error codes [tests/012.phpt] 
PASS PHP Closures from Lua [tests/013.phpt] 
PASS Bug #65097 (nApplyCount release missing) [tests/bug65097.phpt] 
=====================================================================
TIME END 2015-02-04 10:07:50

=====================================================================
TEST RESULT SUMMARY
---------------------------------------------------------------------
Exts skipped    :    0
Exts tested     :   50
---------------------------------------------------------------------

Number of tests :   15                15
Tests skipped   :    0 (  0.0%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    0 (  0.0%) (  0.0%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests leaked    :    0 (  0.0%) (  0.0%)
Tests passed    :   15 (100.0%) (100.0%)
---------------------------------------------------------------------
Time taken      :   49 seconds
=====================================================================
laruence commented 9 years ago

hardcode 100 is un-acceptable

hjanuschka commented 9 years ago

ok you'r right - any idea why the ecalloc call leaks? on test/008.phpt

laruence commented 9 years ago

I will have a look, I am busy with other stuffs, anyway I recently did some changes relates to resource handling. ,, which should affects lua in resource handling, maybe you could have a look? https://github.com/php/php-src/compare/6230493005...77b164edfd

hjanuschka commented 9 years ago

ok got it - problem was the *params took this as a sample for variable sized args https://github.com/php/php-src/blob/fc33f52d8c25997dd0711de3e07d0dc260a18c11/ext/filter/callback_filter.c

rebased the commit - so the arg limit is no more!

test results with -m

=====================================================================
PHP         : /usr/local/bin/php 
PHP_SAPI    : cli
PHP_VERSION : 7.0.0-dev
ZEND_VERSION: 3.0.0-dev
PHP_OS      : Linux - Linux ip-172-31-14-228 3.2.0-4-amd64 #1 SMP Debian 3.2.63-2 x86_64
INI actual  : /home/admin/tmp/php-lua/tmp-php.ini
More .INIs  :  
CWD         : /home/admin/tmp/php-lua
Extra dirs  : 
VALGRIND    : valgrind-3.7.0
=====================================================================
TIME START 2015-02-04 11:57:29
=====================================================================
PASS Basic lua check [tests/001.phpt] 
PASS Check for Table Pass vic-a-verse [tests/0014.phpt] 
PASS Set and read properties [tests/002.phpt] 
PASS Call lua functions [tests/003.phpt] 
PASS Type conversion from lua to PHP [tests/004.phpt] 
PASS Lua phpinfo() block [tests/005.phpt] 
PASS Lua::include() [tests/006.phpt] 
PASS Lua return function [tests/007.phpt] 
PASS register php function to lua [tests/008.phpt] 
PASS Bug (eval and include compute wrong return value number) [tests/009.phpt] 
PASS LuaClosure exception [tests/010.phpt] 
PASS register invalid php callback to lua [tests/011.phpt] 
PASS Lua::include() with error codes [tests/012.phpt] 
PASS PHP Closures from Lua [tests/013.phpt] 
PASS Bug #65097 (nApplyCount release missing) [tests/bug65097.phpt] 
=====================================================================
TIME END 2015-02-04 11:58:17

=====================================================================
TEST RESULT SUMMARY
---------------------------------------------------------------------
Exts skipped    :    0
Exts tested     :   50
---------------------------------------------------------------------

Number of tests :   15                15
Tests skipped   :    0 (  0.0%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    0 (  0.0%) (  0.0%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests leaked    :    0 (  0.0%) (  0.0%)
Tests passed    :   15 (100.0%) (100.0%)
---------------------------------------------------------------------
Time taken      :   48 seconds
=====================================================================
hjanuschka commented 9 years ago

ohh and thx for your resource stuff - lua doesnt seem to be affected - but my other extension is :smile:

i came up with this solution in my php_ext.h

#ifndef ZEND_FETCH_RESOURCE
#define ZEND_FETCH_RESOURCE(o,t,z,i,ln,lb) if((o = (t)zend_fetch_resource(Z_RES_P(z),ln,lb)) == NULL) { RETURN_FALSE }
#endif

#ifndef ZEND_REGISTER_RESOURCE
#define ZEND_REGISTER_RESOURCE(ret,o,l) RETURN_RES(zend_register_resource(o,l))
#endif

wich ports the calls if they aren't defined.

sorry to missuse this PR - can i ask you, if you would help me with getting a pecl account (sponsoring user) for the extension here http://github.com/Bartlby/bartlby-php?

laruence commented 9 years ago

you could try to send a mail to pecl-dev@ php list, if people think your ext is good, then it could be list in PECL...

hjanuschka commented 9 years ago

k - thx - and this PR is going to get merged?

hjanuschka commented 9 years ago

k - rebased!

remove all C99 comments (basicly useless, and reminder how it was before :smiling_imp: ) remove ->ptest (used it on the initial PR to debug Lua_State pointer) removed whitespaces did the ZVAL_COPY_VALUE

all tests pass with -m