when registering a function, their arguments can be dynamically #2462
Conversation
allocated when there is a typed return.
|
@nikic indeed you re right got double frees all the time. |
Zend/zend_API.c
Outdated
| @@ -2473,6 +2475,12 @@ ZEND_API void zend_unregister_functions(const zend_function_entry *functions, in | |||
| fname_len = strlen(ptr->fname); | |||
| lowercase_name = zend_string_alloc(fname_len, 0); | |||
| zend_str_tolower_copy(ZSTR_VAL(lowercase_name), ptr->fname, fname_len); | |||
| reg_function = zend_hash_find_ptr(target_function_table, lowercase_name); | |||
| if (reg_function->common.arg_info && | |||
There was a problem hiding this comment.
This should also have a reg_function && chechk, because zend_unregister_functions is also called in error cases where not all functions have been defined, for example: https://github.com/devnexen/php-src/blob/f3ee93de6574c479515519c4e49271777f383264/Zend/zend_API.c#L2236
|
Hmm, unfortunately this doesn't fix the leak completely: class methods aren't explicitly unregistered (instead the function table is simply destroyed), so those are still leaking. |
|
What's the plan here ? Seems like it needs more work or another approach ? |
|
Yeah, I'm not totally sure what the best way to fix this is. Maybe creating the functions with a FREE_ARGINFO flag, and dropping that flag in function copying might work. |
|
do you think at least merging this would suffice for function part and making separated PR for classes would be good enough ? |
|
I'm going to close this one as it's quite clear we need a different and complete approach that solves the problem in general. Sorry it took so long ... |
allocated when there is a typed return.