-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Allow using fast destruction path when ASAN is in use #18835
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Ping @nielsdos |
By design, fast shutdown does not explicitly free all request data but defers that to ZendMM. If ZendMM is not used, but you use fast shutdown, then you get real leaks. |
A compromise could be to make it opt-in when |
Zend/zend_execute_API.c
Outdated
#elif defined(__SANITIZE_ADDRESS__) | ||
bool fast_shutdown = !EG(full_tables_cleanup); | ||
#else | ||
bool fast_shutdown = is_zend_mm() && !EG(full_tables_cleanup); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fast_shutdown relays on Zend MM that frees the whole request heap at once. It doesn't make sense without it and will cause memory leaks.
Makes sense, added a ZEND_FORCE_FAST_SHUTDOWN envvar conditional: leak detection must be disabled when using this option, i.e.
Detects #18833 without emitting redundant leak issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a debugging feature, in combination with ASAN_OPTIONS=detect_leaks=0
, I can get behind this.
Disabling the fast path when a custom allocator is in use hid the #18833 issue, and made it super hard to debug, this re-enables it if php is compiled with asan.