PHP7 support #27
PHP7 support #27
Conversation
PHP7 no longer coerces hex strings to integers
|
Thanks for doing this, including the Travis changes! The diff is pretty big so it might take us a little while to review this. Do you know of any good documentation on the changes to Zend Engine that accompany PHP 7? I briefly looked and did not find anything great. From your changes, they appear to be reasonably significant. |
|
@oschwald Yep. Here's the documentation: Here's a few third-party extensions that have been working on dual PHP5/PHP7 support: |
|
Great. Thanks! |
| FREE_HASHTABLE(obj->std.properties); | ||
| } | ||
|
|
||
| //efree(obj); |
There was a problem hiding this comment.
Why is this no longer necessary? If it is really not necessary, could we remove it rather than commenting it out?
There was a problem hiding this comment.
I'm not exactly sure, it may have to do with the refcounting changes. It seems to be standard in extensions though:
https://github.com/php/php-src/blob/php-5.6.5/ext/mysqli/mysqli.c#L214
https://github.com/php/php-src/blob/master/ext/mysqli/mysqli.c#L205
https://github.com/php/php-src/blob/php-5.6.5/ext/dom/php_dom.c#L1088
https://github.com/php/php-src/blob/master/ext/dom/php_dom.c#L1026
| } | ||
|
|
||
| static zend_class_entry *lookup_class(const char *name TSRMLS_DC) | ||
| { | ||
| #ifdef ZEND_ENGINE_3 | ||
| zend_string *n = zend_string_init(name, strlen(name), 0); |
There was a problem hiding this comment.
I am getting leaks here when running the test suite with Valgrind on PHP 7:
==24951== by 0xFD0554E: zend_string_alloc (zend_string.h:121)
==24951== by 0xFD0554E: zend_string_init (zend_string.h:157)
==24951== by 0xFD0554E: lookup_class (maxminddb.c:458)
==24951== by 0xFD05FFB: zim_MaxMind_Db_Reader_close (maxminddb.c:291)
==24951== by 0x85B621: ZEND_DO_FCALL_SPEC_HANDLER (zend_vm_execute.h:836)
==24951== by 0x80705A: execute_ex (zend_vm_execute.h:406)
==24951== by 0x7B61C9: zend_call_function (zend_execute_API.c:852)
==24951== by 0x68DDC5: zim_reflection_method_invokeArgs (php_reflection.c:3365)
==24951== by 0x85B621: ZEND_DO_FCALL_SPEC_HANDLER (zend_vm_execute.h:836)
==24951== by 0x80705A: execute_ex (zend_vm_execute.h:406)
==24951== by 0x85EE86: zend_execute (zend_vm_execute.h:450)
==24951== by 0x7C58C4: zend_execute_scripts (zend.c:1404)
==24951== by 0x7652CF: php_execute_script (main.c:2475)
There was a problem hiding this comment.
Weird, I can't get anything to produce that or the above message even though it should obviously leak. I've been running:
TEST_PHP_ARGS=-m make test and USE_ZEND_ALLOC=0 valgrind --leak-check=full ~/build/php7/bin/php -n -dextension=ext/modules/maxminddb.so -dreport_memleaks=On ./vendor/bin/phpunit
There was a problem hiding this comment.
I think you need to set ZEND_DONT_UNLOAD_MODULES=1 as otherwise the module is unloaded and the relevant frames are not included in the trace.
There was a problem hiding this comment.
@oschwald yep, that did it, thanks!
|
Thanks! With the latest commit, this looks great. 👍 |
Some notes: