Skip to content
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

fix memleak due to missing pthread_attr_destroy()-call #14510

Closed

Conversation

realFlowControl
Copy link
Contributor

@realFlowControl realFlowControl commented Jun 8, 2024

Hey there 👋

I just added PHP 8.3 with ASAN to ext-parallel and it is failing. The stack traces I attached led me to the docs of the pthread_getattr_np() call where it is stated that:

When the thread attributes object returned by pthread_getattr_np() is no longer required, it should be destroyed using pthread_attr_destroy(3).

The implementation of pthread_attr_setaffinity_np() (which is called by pthread_getattr_np()) does some allocations, that are freed in the pthread_attr_destroy() function, so I added this call. It looks like this code is related to #9104, so I'd appreciate a review from @arnaud-lb.

With the proposed changes, ASAN in ext-parallel is green again. I can not rule out, that I missed something, or that I am just misusing the API in some way.

This is the stack trace ASAN is showing locally:

Direct leak of 152 byte(s) in 1 object(s) allocated from:
    #0 0xffff96b0a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0xffff9688ad30  (/lib/aarch64-linux-gnu/libc.so.6+0x7ad30)
    #2 0xffff9688b098 in pthread_attr_setaffinity_np (/lib/aarch64-linux-gnu/libc.so.6+0x7b098)
    #3 0xffff9688e7b0 in pthread_getattr_np (/lib/aarch64-linux-gnu/libc.so.6+0x7e7b0)
    #4 0xaaaabe8c9820 in zend_call_stack_get_linux_pthread /opt/src/php-src/Zend/zend_call_stack.c:120
    #5 0xaaaabe8c9e28 in zend_call_stack_get_linux /opt/src/php-src/Zend/zend_call_stack.c:239
    #6 0xaaaabe8c9e98 in zend_call_stack_get /opt/src/php-src/Zend/zend_call_stack.c:511
    #7 0xaaaabe8c9078 in zend_call_stack_init /opt/src/php-src/Zend/zend_call_stack.c:63
    #8 0xaaaabe9896d4 in zend_new_thread_end_handler /opt/src/php-src/Zend/zend.c:848
    #9 0xaaaabe75d030 in allocate_new_resource /opt/src/php-src/TSRM/TSRM.c:412
    #10 0xaaaabe75d39c in ts_resource_ex /opt/src/php-src/TSRM/TSRM.c:461
    #11 0xffff9377e1f8  (<unknown module>)
    #12 0xffff93780b88  (<unknown module>)
    #13 0xffff9688d5c4  (/lib/aarch64-linux-gnu/libc.so.6+0x7d5c4)
    #14 0xffff968f5ed8  (/lib/aarch64-linux-gnu/libc.so.6+0xe5ed8)

Indirect leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0xffff96b0a69c in __interceptor_realloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:164
    #1 0xffff9688b0bc in pthread_attr_setaffinity_np (/lib/aarch64-linux-gnu/libc.so.6+0x7b0bc)
    #2 0xffff9688e7b0 in pthread_getattr_np (/lib/aarch64-linux-gnu/libc.so.6+0x7e7b0)
    #3 0xaaaabe8c9820 in zend_call_stack_get_linux_pthread /opt/src/php-src/Zend/zend_call_stack.c:120
    #4 0xaaaabe8c9e28 in zend_call_stack_get_linux /opt/src/php-src/Zend/zend_call_stack.c:239
    #5 0xaaaabe8c9e98 in zend_call_stack_get /opt/src/php-src/Zend/zend_call_stack.c:511
    #6 0xaaaabe8c9078 in zend_call_stack_init /opt/src/php-src/Zend/zend_call_stack.c:63
    #7 0xaaaabe9896d4 in zend_new_thread_end_handler /opt/src/php-src/Zend/zend.c:848
    #8 0xaaaabe75d030 in allocate_new_resource /opt/src/php-src/TSRM/TSRM.c:412
    #9 0xaaaabe75d39c in ts_resource_ex /opt/src/php-src/TSRM/TSRM.c:461
    #10 0xffff9377e1f8  (<unknown module>)
    #11 0xffff93780b88  (<unknown module>)
    #12 0xffff9688d5c4  (/lib/aarch64-linux-gnu/libc.so.6+0x7d5c4)
    #13 0xffff968f5ed8  (/lib/aarch64-linux-gnu/libc.so.6+0xe5ed8)

SUMMARY: AddressSanitizer: 184 byte(s) leaked in 2 allocation(s).

@realFlowControl
Copy link
Contributor Author

The failing ASAN test is the same that's failing in PHP-8.3 branch from where I branched off and seems unrelated to me.

Copy link
Member

@arnaud-lb arnaud-lb left a comment

Choose a reason for hiding this comment

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

This looks good to me apart from one comment

Zend/zend_call_stack.c Outdated Show resolved Hide resolved
@arnaud-lb
Copy link
Member

Thank you!

@realFlowControl realFlowControl deleted the florian/zts-mem-leak branch June 10, 2024 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants