Bug #75633 [Opn]: In multi-threaded code, if a thread handle is reused by the OS, the app crashes

Edit report at https://bugs.php.net/bug.php?id=75633&edit=1 ID: 75633 Updated by: ab@php.net Reported by: rperper at litespeedtech dot com Summary: In multi-threaded code, if a thread handle is reused by the OS, the app crashes Status: Open Type: Bug Package: Reproducible crash Operating System: OpenSuSE PHP Version: 7.2.0 Block user comment: N Private report: N New Comment: Thanks for the tests. Actually, that's an interesting experiment, from the usability perspective it seems however to make a little sense. Once a thread is started, it serves multiple requests, not just one. The most meaningful parallel in this case were probably to check how Apache handles it with MinSpareThreads/MaxSpareThreads. There might be also different handling in regard to whether an MPM uses multiple processes, but i haven't seen so far that threads would be joined until the process exit. When looking it from the MPM implementation perspective - it seems also logic, as a MPM cannot know about implementation details of a concrete SAPI module. Until MPM also doesn't provide callback for thread cleanups, it should not join threads for this reason. Now, I don't know the exact features the litespeed threading provides, but with the regard to the current TSRM implementation the test code doesn't seem correct. Once a thread() is called, it simply should start a new serving instance. After that it can accept clients and serve multiple requests per process_req(). Furthermore, the lone thread(NULL) calls in the test code look wrong, they should only be called in the new thread context so the correct TLS items are used. At this point - perhaps the TSRM layer could be extended with the thread cleanup callbacks for the case a server module provides such a functionality, but otherwise the functionality is OK when sage of the TSRM API is sufficient. Thanks. Previous Comments: ------------------------------------------------------------------------ [2017-12-05 15:17:19] rperper at litespeedtech dot com I would have expected that it would be seen more often as well. But one of the people here suggested that perhaps other developers create a thread and reuse it, only creating new threads for larger amounts of work - and not deleting the existing ones. I don't see a lot of thread questions and bugs posted so I can't speak to how much threads are used. Except that I was told that 5.x threading is no longer supported and not until 7.2 has threading really been functional. Thanks, Bob rperper@litespeedtech.com ------------------------------------------------------------------------ [2017-12-05 14:56:10] danack@php.net interesting bug, a possibly stupid question: Any idea why isn't this a more commonly seen problem? I would have imagined if the code wasn't handling that scenario correctly, then other servers that dynamically allocate threads would have seen similar issues. ------------------------------------------------------------------------ [2017-12-05 14:35:40] rperper at litespeedtech dot com Description: ------------ I am a developer at LiteSpeed Technologies and am working on a thread-capable version of the PHP module to be included in the Open-LiteSpeed web server. During load testing, our application would occasionally crash and always at the same place: at a "zend_first_try". In testing, the crash would occur just by reading the value in EG(bailout) (the first thing done by zend_first_try). After quite a bit of examination, it was determined that a thread was being created, destroyed, a new thread was created and it had the same pthread_self value. In examining TSRM.c it appears that the thread_self value is hashed to reference globals. This will fail if the thread_self value is reused (which it is by the operating system). I have recreated the problem with a test program which is included. Test script: --------------- Install in a php 7.2.0 installation in the sapi directory: https://drive.google.com/file/d/1VK7tcSq3zk-DFNF678OQSWxo03QPpmaK/view?usp=sharing Copy the tar file to that directory and extract it: tar xvf phptest.tar The instructions on how to compile and test it are in the README. But it basically comes down to running ./buildconf --force, configure and make. The crash will occur on execution of the generated phptest program. Expected result: ---------------- Either have a method to automatically detect that a pthread_self value is an actual different thread (which can be done using the syscall interface) or provide a function for us to call to clear out a thread's details when we destroy the thread. Actual result: -------------- Backtrace in gdb: #0 process_req() at /home/user/proj/phptest/php-7.2.0/sapi/phptest/phptest.c:392 #1 begin_process() at /home/user/proj/phptest/php-7.2.0/sapi/phptest/phptest.c:423 #2 thread() at /home/user/proj/phptest/php-7.2.0/sapi/phptest/phptest.c:429 #3 start_thread() at /lib64/libpthread.so.0 #4 clone() at /lib64/libc.so.6 ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=75633&edit=1