Edit report at https://bugs.php.net/bug.php?id=62991&edit=1
ID: 62991 Patch added by: larue...@php.net Reported by: softwareelves at gmail dot com Summary: Segfault with generator and closure. Status: Assigned Type: Bug Package: Reproducible crash Operating System: Mac OSx 10.8.1 PHP Version: master-Git-2012-09-02 (Git) Assigned To: nikic Block user comment: N Private report: N New Comment: The following patch has been added/updated: Patch Name: bug62991.phpt Revision: 1346586639 URL: https://bugs.php.net/patch-display.php?bug=62991&patch=bug62991.phpt&revision=1346586639 Previous Comments: ------------------------------------------------------------------------ [2012-09-02 11:46:56] larue...@php.net a new patch has been attached, fixed the memleak issue, but the way is a little tricky, used the op_array->reserved fields. so I attached it here instead of ci it, wait for if we can find a better way ------------------------------------------------------------------------ [2012-09-02 11:45:06] larue...@php.net The following patch has been added/updated: Patch Name: bug62991.patch Revision: 1346586306 URL: https://bugs.php.net/patch-display.php?bug=62991&patch=bug62991.patch&revision=1346586306 ------------------------------------------------------------------------ [2012-09-02 11:24:00] larue...@php.net okey, but is there a way to find out that whether a generator has been run once? leaks reporting if the closure didn't run. ------------------------------------------------------------------------ [2012-09-02 10:26:03] ni...@php.net Oh, and also, I think it would be a little bit nicer if this: + if (execute_data->op_array->fn_flags & ZEND_ACC_CLOSURE) { + destroy_op_array(execute_data->op_array); + efree(execute_data->op_array); + } would be written as: + if (op_array->fn_flags & ZEND_ACC_CLOSURE) { + destroy_op_array(op_array); + efree(op_array); + } There already is a local op_array variable for execute_data->op_array, so it's a bit shorter to use ;) ------------------------------------------------------------------------ [2012-09-02 10:23:04] ni...@php.net @laruence: The patch looks fine for me. The only thing that looks strange are these whitespace changes: -ZEND_BEGIN_ARG_INFO_EX(arginfo_closure_bindto, 0, 0, 1) + ZEND_BEGIN_ARG_INFO_EX(arginfo_closure_bindto, 0, 0, 1) ZEND_ARG_INFO(0, newthis) ZEND_ARG_INFO(0, newscope) ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_INFO_EX(arginfo_closure_bind, 0, 0, 2) + ZEND_BEGIN_ARG_INFO_EX(arginfo_closure_bind, 0, 0, 2) ZEND_ARG_INFO(0, closure) ZEND_ARG_INFO(0, newthis) ZEND_ARG_INFO(0, newscope) ZEND_END_ARG_INFO() -static const zend_function_entry closure_functions[] = { - ZEND_ME(Closure, __construct, NULL, ZEND_ACC_PRIVATE) - ZEND_ME(Closure, bind, arginfo_closure_bind, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) - ZEND_MALIAS(Closure, bindTo, bind, arginfo_closure_bindto, ZEND_ACC_PUBLIC) - {NULL, NULL, NULL} -}; + static const zend_function_entry closure_functions[] = { + ZEND_ME(Closure, __construct, NULL, ZEND_ACC_PRIVATE) + ZEND_ME(Closure, bind, arginfo_closure_bind, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) + ZEND_MALIAS(Closure, bindTo, bind, arginfo_closure_bindto, ZEND_ACC_PUBLIC) + {NULL, NULL, NULL} + }; Looks like the indentation is slightly off there :) ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=62991 -- Edit this bug report at https://bugs.php.net/bug.php?id=62991&edit=1