Bug #68486 [Com]: PHP SegFault zend_hash_find

Edit report at https://bugs.php.net/bug.php?id=68486&edit=1 ID: 68486 Comment by: gmoniker at gmail dot com Reported by: wattwood at tcstire dot com Summary: PHP SegFault zend_hash_find Status: Open Type: Bug Package: Apache2 related Operating System: Ubuntu 14.04.1 LTS PHP Version: 5.5.19 Block user comment: N Private report: N New Comment: It seems to me that it will be hard to revalidate this handler if it is changed extensively. Just did some testing with a one line adjustment to the code of "sapi/apache2handler/sapi_apache2.c". Towards the end of php_handler(request_rec *r) I included the line: apr_pool_cleanup_run(r->pool, (void *)&SG(server_context), php_server_context_cleanup); So it becomes: if (!parent_req) { php_apache_request_dtor(r TSRMLS_CC); ctx->request_processed = 1; bucket = apr_bucket_eos_create(r->connection->bucket_alloc); APR_BRIGADE_INSERT_TAIL(brigade, bucket); rv = ap_pass_brigade(r->output_filters, brigade); if (rv != APR_SUCCESS || r->connection->aborted) { zend_first_try { php_handle_aborted_connection(); } zend_end_try(); } apr_brigade_cleanup(brigade); apr_pool_cleanup_run(r->pool, (void *)&SG(server_context), php_server_context_cleanup); } else { ctx->r = parent_req; } It seems to me this solves the missing automatic request pool destruction after a request in Apache 2.4 leading the php_handler routine to consider independent follow-on requests a subrequest of the first request when they are not. The request pool can continue on, but without the php_struct in it. I am afraid the Apache 2.4 handling of these pipelines can cause memory bloat of the server process, but that belongs in Apaches corner. Previous Comments: ------------------------------------------------------------------------ [2015-03-17 16:42:24] php at bof dot de ab@php.net : regarding reproducing the issue you can take any PHP script, a simple one-line echo is sufficient, and then request it twice using the echo|netcat command as shown in several of the comments here. ------------------------------------------------------------------------ [2015-03-17 16:38:32] php at bof dot de Sorry for the broken gist patches. Apparently gists do funny things with whitespace... I should have tested that, but didn't. To make things easy I put the patches up on my personal webspace. Please don't link to that in any blog posts or stuff like that :) Same patch as in the last gist, without and with added debug/trace calls: http://bei.bof.de/php/sapi_apache2.noreentry.v2.patch http://bei.bof.de/php/sapi_apache2.noreentry.debug.v2.patch Since then I also added something alluded to on internals in a new thread "PHP apache2handler virtual() function" - now implemented with a new PHP function apache_tail_request(). This can be found, on top of the changes of the v2 patch, in these patches: http://bei.bof.de/php/sapi_apache2.noreentry.v3.patch http://bei.bof.de/php/sapi_apache2.noreentry.debug.v3.patch All patches were made on top of PHP 5.6.7RC1, checked out by tag from git. For about 2 hours now, I have the v3 patch actively running on one of my production servers, under apache 2.4 now, and without any coredumps / segvs in the logs or any kind of performance regression. This is with some 80 requests per second, so it gets a good workout. The patch appears to work. ------------------------------------------------------------------------ [2015-03-16 19:11:22] gmoniker at gmail dot com This is some great work. One remark, the php_functions.c file of the apache2handler inside the virtual definition mentions that the ap_rflush is supposed to be a workaround for http://issues.apache.org/bugzilla/show_bug.cgi?id=17629. Fortunately reading its comments seems to indicate that the underlying problem was solved in Apache 2.2 around august 2010 with https://svn.apache.org/viewvc?view=revision&revision=988400. ------------------------------------------------------------------------ [2015-03-16 14:11:00] ab@php.net Patrick, somehow the latest patch is broken, it won't apply to any of 5.5 through master. Also, is there any piece of code to be used for the tests? Thanks. ------------------------------------------------------------------------ [2015-03-15 10:17:28] php at bof dot de Reworked the patch another round, see updated gist: https://gist.github.com/bof/15173c7a11cb12a7b96f The pool cleanup logic is reworked, with the goal of never cleaning up the SG(server_context) of something other than the currently running + not yet complete request - regardless of when and how delayed a pool cleanup callback comes in. This I do by 1) adding a ctx->server_context pointer to the context structure, 2) registering ctx instead of &SG(server_context) with the pool cleanup callback, and finally 3) in php_server_context_cleanup() I check that *ctx->server_context == ctx, and only in that case I NULL it. I also rearranged the php_handler() config handling a bit, so that the xbithack-during-reentry case should now be correct, too; and there's some all-around cleanup. The debug output of the previous patch is gone (available on request). I tested this patch, lightly, with my testcases and our normal codebase, under both Apache 2.4.12, the openSUSE 13.1 Apache 2.4.6, and an older Apache 2.2 setup as I use it in normal production so far. No issues were found, the new logic seems to work the same everywhere. I also ran the apache 2.4.12 variant under valgrind, also without any issues flagged. THIS STILL NEEDS REVIEW from somebody who previously worked on the apache2handler code. But I think it's solid enough now, and I'll probably test it in production on monday ------------------------------------------------------------------------ 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=68486 -- Edit this bug report at https://bugs.php.net/bug.php?id=68486&edit=1