Skip to content

Memleaks - #16

Merged
laruence merged 1 commit into
laruence:masterfrom
hjanuschka:hj
Feb 4, 2015
Merged

laruence merged 1 commit into
laruence:masterfrom
hjanuschka:hj

Conversation

@hjanuschka

Copy link
Copy Markdown
Contributor

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

laruence commented Feb 4, 2015

Copy link
Copy Markdown
Owner

hardcode 100 is un-acceptable

@hjanuschka

Copy link
Copy Markdown
Contributor Author

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

@laruence

laruence commented Feb 4, 2015

Copy link
Copy Markdown
Owner

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? php/php-src@6230493...77b164e

@hjanuschka

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor Author

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

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 https://github.com/Bartlby/bartlby-php?

@laruence

laruence commented Feb 4, 2015

Copy link
Copy Markdown
Owner

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

Copy link
Copy Markdown
Contributor Author

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

Comment thread lua.c Outdated

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please do not use c99 style comment , use /* */

@hjanuschka
hjanuschka force-pushed the hj branch 2 times, most recently from 48c5de6 to 5e5fefc Compare February 4, 2015 12:53
* no more outstanding memory leaks
@hjanuschka

Copy link
Copy Markdown
Contributor Author

k - rebased!

remove all C99 comments (basicly useless, and reminder how it was before 😈 )
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

laruence added a commit that referenced this pull request Feb 4, 2015
@laruence
laruence merged commit bcc3bb6 into laruence:master Feb 4, 2015
@zhang-bin zhang-bin mentioned this pull request Feb 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants