Bug #75633 [Csd]: 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: Closed Type: Bug Package: Reproducible crash Operating System: OpenSuSE PHP Version: 7.2.0 -Assigned To: +Assigned To: ab Block user comment: N Private report: N New Comment: Even better then :) Previous Comments: ------------------------------------------------------------------------ [2017-12-06 18:55:59] rperper at litespeedtech dot com We were able to use the existing ts_free_thread() call and it seems to have addressed our problem. I'm closing the problem as we're happy here. Thanks again, Bob Perper rperper@litespeedtech.com ------------------------------------------------------------------------ [2017-12-06 17:50:56] ab@php.net @rperper please lets see the patch first then. Whether it's suitable for a patch version depends on the extent and severity. As general principles, a patch that targets a stable branch has to retain BC and not breach binary compatibility. Obviously it also should be ensured CLI and Apache still work. Depending on all that, an approval through RM could be required. Otherwise, you could target master. Thanks. ------------------------------------------------------------------------ [2017-12-06 14:10:02] rperper at litespeedtech dot com We consider this to be a critical path problem as there's no reasonable way in our code to have a thread never terminate. So we'll be generating a patch to allow cleanup of resources when a thread terminates and posting it to this problem within the next couple of days. We really hope it can be included in the next PHP patch release. Please let me know if this is a problem for you. Thanks much. Bob Perper rperper@litespeedtech.com ------------------------------------------------------------------------ [2017-12-06 11:10:21] ab@php.net 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. ------------------------------------------------------------------------ [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 ------------------------------------------------------------------------ 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=75633 -- Edit this bug report at https://bugs.php.net/bug.php?id=75633&edit=1