r103433 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r103432‎ | r103433 | r103434 >
Date:01:27, 17 November 2011
Author:tstarling
Status:deferred
Tags:zend 
Comment:
* Added a workaround for https://bugs.php.net/bug.php?id=31749 . Do a flush and abort after timeout instead of returning control to Apache, and set a timer which will protect against deadlocks within wmerrors itself.
* Added user function wmerrors_malloc_test() which is a tight infinite loop of malloc() intended to allow easy testing of libc deadlocks.
Modified paths:
  • /trunk/php/wmerrors/README (modified) (history)
  • /trunk/php/wmerrors/debian/changelog (modified) (history)
  • /trunk/php/wmerrors/php_wmerrors.h (modified) (history)
  • /trunk/php/wmerrors/wmerrors.c (modified) (history)

Diff [purge]

Index: trunk/php/wmerrors/wmerrors.c
——@@ -3,6 +3,8 @@
44 #include "config.h"
55 #endif
66
 7+#include <stdlib.h>
 8+
79 #include "php.h"
810 #include "php_ini.h"
911 #include "php_wmerrors.h"
——@@ -12,15 +14,25 @@
1315 #include "ext/standard/php_smart_str.h" /* for smart_str */
1416 #include "Zend/zend_builtin_functions.h" /* for zend_fetch_debug_backtrace */
1517
 18+static int wmerrors_post_deactivate();
1619 static void wmerrors_cb(int type, const char *error_filename, const uint error_lineno, const char *format, va_list args);
1720 static void wmerrors_show_message(int type, const char *error_filename, const uint error_lineno, const char *format, va_list args TSRMLS_DC);
1821 static void wmerrors_get_concise_backtrace(smart_str *s TSRMLS_DC);
1922 static void wmerrors_write_full_backtrace(php_stream *logfile_stream);
2023 static void wmerrors_write_request_info(php_stream *logfile_stream TSRMLS_DC);
 24+static void wmerrors_alarm_handler(int signo);
 25+static void wmerrors_install_alarm(TSRMLS_D);
 26+static void wmerrors_remove_alarm(TSRMLS_D);
2127
2228 ZEND_DECLARE_MODULE_GLOBALS(wmerrors)
2329
 30+PHP_FUNCTION(wmerrors_malloc_test);
 31+
 32+ZEND_BEGIN_ARG_INFO(wmerrors_malloc_test_arginfo, 0)
 33+ZEND_END_ARG_INFO()
 34+
2435 zend_function_entry wmerrors_functions[] = {
 36+ PHP_FE(wmerrors_malloc_test, wmerrors_malloc_test_arginfo)
2537 {NULL, NULL, NULL}
2638 };
2739
——@@ -39,7 +51,9 @@
4052 #if ZEND_MODULE_API_NO >= 20010901
4153 "1.1.2",
4254 #endif
43 - STANDARD_MODULE_PROPERTIES
 55+ NO_MODULE_GLOBALS,
 56+ wmerrors_post_deactivate,
 57+ STANDARD_MODULE_PROPERTIES_EX
4458 };
4559
4660
——@@ -54,6 +68,7 @@
5569 STD_PHP_INI_BOOLEAN("wmerrors.log_backtrace", "0", PHP_INI_ALL, OnUpdateBool, log_backtrace, zend_wmerrors_globals, wmerrors_globals)
5670 STD_PHP_INI_BOOLEAN("wmerrors.ignore_logging_errors", "0", PHP_INI_ALL, OnUpdateBool, ignore_logging_errors, zend_wmerrors_globals, wmerrors_globals)
5771 STD_PHP_INI_BOOLEAN("wmerrors.backtrace_in_php_error_message", "0", PHP_INI_ALL, OnUpdateBool, backtrace_in_php_error_message, zend_wmerrors_globals, wmerrors_globals)
 72+ STD_PHP_INI_ENTRY("wmerrors.timeout", "10", PHP_INI_ALL, OnUpdateLong, timeout, zend_wmerrors_globals, wmerrors_globals)
5873 PHP_INI_END()
5974
6075 void (*old_error_cb)(int type, const char *error_filename, const uint error_lineno, const char *format, va_list args);
——@@ -87,6 +102,7 @@
88103 PHP_RINIT_FUNCTION(wmerrors)
89104 {
90105 WMERRORS_G(recursion_guard) = 0;
 106+ WMERRORS_G(alarm_set) = 0;
91107 return SUCCESS;
92108 }
93109
——@@ -97,6 +113,13 @@
98114 return SUCCESS;
99115 }
100116
 117+int wmerrors_post_deactivate()
 118+{
 119+ TSRMLS_FETCH();
 120+ wmerrors_remove_alarm(TSRMLS_C);
 121+ return SUCCESS;
 122+}
 123+
101124 PHP_MINFO_FUNCTION(wmerrors)
102125 {
103126 php_info_print_table_start();
——@@ -143,6 +166,10 @@
144167 /* No more OOM errors for now thanks */
145168 zend_set_memory_limit((size_t)-1);
146169
 170+ if (WMERRORS_G(timeout)) {
 171+ wmerrors_install_alarm(TSRMLS_C);
 172+ }
 173+
147174 /* Do not show the html error to console */
148175 if ( WMERRORS_G(enabled) && strncmp(sapi_module.name, "cli", 3) ) {
149176 /* Show the message */
——@@ -155,18 +182,27 @@
156183 }
157184
158185 /* Put a concise backtrace in the normal output */
159 - if (WMERRORS_G(backtrace_in_php_error_message))
 186+ if (WMERRORS_G(backtrace_in_php_error_message)) {
160187 wmerrors_get_concise_backtrace(&new_filename TSRMLS_CC);
 188+ }
161189 smart_str_appendl(&new_filename, error_filename, strlen(error_filename));
162190 smart_str_0(&new_filename);
163191
164192 WMERRORS_G(recursion_guard) = 0;
165193 zend_set_memory_limit(PG(memory_limit));
166194
 195+ if (PG(connection_status) & PHP_CONNECTION_TIMEOUT) {
 196+ /* abort instead of deadlocking */
 197+ const char abort_message[] = "wmerrors: doing precautionary abort() after request timeout\n";
 198+ write(STDERR_FILENO, abort_message, sizeof(abort_message) - 1);
 199+ abort();
 200+ }
 201+
167202 /* Pass through */
168203 old_error_cb(type, new_filename.c, error_lineno, format, args);
169204
170 - /* Note: old_error_cb() may not return, in which case there will be no explicit free of new_filename */
 205+ /* Note: old_error_cb() may not return, in which case there will be no
 206+ * explicit free of new_filename */
171207 smart_str_free(&old_error_cb);
172208 }
173209
——@@ -480,7 +516,13 @@
481517 /* Write the message out */
482518 if (expanded.c) {
483519 php_write(expanded.c, expanded.len TSRMLS_CC);
 520+
 521+ if (PG(connection_status) & PHP_CONNECTION_TIMEOUT) {
 522+ /* Probably PHP will crash soon, better flush the output first */
 523+ sapi_flush(TSRMLS_C);
 524+ }
484525 }
 526+
485527
486528 /* Clean up */
487529 smart_str_free(&expanded);
——@@ -521,4 +563,58 @@
522564 }
523565 }
524566
 567+/**
 568+ * Occasionally, PHP will time out during a malloc() call, generating SIGALRM.
 569+ * A mutex will be held, and then the process will deadlock forever on the
 570+ * next malloc() call. This timeout handler is designed to handle this
 571+ * situation safely.
 572+ *
 573+ * We could try to avoid using malloc(), but then the process would deadlock
 574+ * when it returns control to Apache.
 575+ */
 576+static void wmerrors_alarm_handler(int signo) {
 577+ const char message[] = "wmerrors: timed out during fatal error handler, aborting\n";
 578+ write(STDERR_FILENO, message, sizeof(message) - 1);
 579+ abort();
 580+}
525581
 582+static void wmerrors_install_alarm(TSRMLS_D) {
 583+#ifdef WMERRORS_USE_TIMER
 584+ struct sigaction sa;
 585+ struct sigevent evp;
 586+ struct itimerspec its;
 587+
 588+ memset(&sa, sizeof(sa), 0);
 589+ sa.sa_handler = wmerrors_alarm_handler;
 590+ sigaction(SIGRTMIN+3, &sa, &WMERRORS_G(old_rt_action));
 591+
 592+ evp.sigev_notify = SIGEV_SIGNAL;
 593+ evp.sigev_signo = SIGRTMIN+3;
 594+ if (timer_create(CLOCK_REALTIME, &evp, &WMERRORS_G(timer)) == -1) {
 595+ perror("timer_create");
 596+ abort();
 597+ }
 598+
 599+ its.it_value.tv_sec = WMERRORS_G(timeout);
 600+ its.it_value.tv_nsec = 0;
 601+ its.it_interval.tv_sec = its.it_interval.tv_nsec = 0;
 602+ timer_settime(WMERRORS_G(timer), 0, &its, NULL);
 603+ WMERRORS_G(alarm_set) = 1;
 604+#endif
 605+}
 606+
 607+static void wmerrors_remove_alarm(TSRMLS_D) {
 608+#ifdef WMERRORS_USE_TIMER
 609+ if (WMERRORS_G(alarm_set)) {
 610+ timer_delete(WMERRORS_G(timer));
 611+ sigaction(SIGRTMIN+3, &WMERRORS_G(old_rt_action), NULL);
 612+ WMERRORS_G(alarm_set) = 0;
 613+ }
 614+#endif
 615+}
 616+
 617+PHP_FUNCTION(wmerrors_malloc_test) {
 618+ for (;;) {
 619+ free(malloc(100));
 620+ }
 621+}
Index: trunk/php/wmerrors/debian/changelog
——@@ -1,3 +1,11 @@
 2+php5-wmerrors (1.1.3-1) oneiric; urgency=low
 3+
 4+ * Added a workaround for https://bugs.php.net/bug.php?id=31749 . Do a flush
 5+ and abort after timeout instead of returning control to Apache, and set a
 6+ timer which will protect against deadlocks within wmerrors itself.
 7+
 8+ -- Tim Starling <tstarling@wikimedia.org> Thu, 17 Nov 2011 12:11:43 +1100
 9+
210 php5-wmerrors (1.1.2-2) lucid; urgency=low
311
412 * Recompiled against new version of php5
Index: trunk/php/wmerrors/README
——@@ -62,3 +62,10 @@
6363 If this is true, a concise backtrace, listing base filenames (not including
6464 the path) and line numbers only, will be included in the error message
6565 which is passed through to PHP, for output into error_log.
 66+
 67+wmerrors.timeout
 68+
 69+ A timeout for the fatal error handler and part of the subsequent PHP
 70+ request shutdown. PHP is prone to deadlocks during fatal error handling
 71+ (bug 31749), this feature is intended to mitigate their impact. If the
 72+ timeout expires, a message is written to stderr and abort() is called.
Index: trunk/php/wmerrors/php_wmerrors.h
——@@ -17,6 +17,12 @@
1818
1919 #include "ext/standard/php_smart_str_public.h"
2020
 21+#if _POSIX_C_SOURCE >= 200112 && !defined(ZTS)
 22+#define WMERRORS_USE_TIMER
 23+#include <signal.h>
 24+#include <time.h>
 25+#endif
 26+
2127 PHP_MINIT_FUNCTION(wmerrors);
2228 PHP_MSHUTDOWN_FUNCTION(wmerrors);
2329 PHP_RINIT_FUNCTION(wmerrors);
——@@ -31,6 +37,14 @@
3238 int log_backtrace;
3339 int ignore_logging_errors;
3440 int backtrace_in_php_error_message;
 41+ long timeout;
 42+ int alarm_set;
 43+ void (*old_on_timeout)(int seconds TSRMLS_DC);
 44+#ifdef WMERRORS_USE_TIMER
 45+ struct sigaction old_rt_action;
 46+ timer_t timer;
 47+#endif
 48+
3549 ZEND_END_MODULE_GLOBALS(wmerrors)
3650
3751

Sign-offs

UserFlagDate
😂inspected22:13, 14 January 2012

Status & tagging log