From: Mitch Hagstrand Date: Wed, 12 Oct 2016 01:18:59 +0000 (-0500) Subject: Fixed bug in zend_accel_error() and cleaned up kill_all_lockers() X-Git-Tag: php-7.1.0RC4~21^2~11 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=bcee2fdbec4f4bba59d4134003cfaf5b1f9b67ab;p=php Fixed bug in zend_accel_error() and cleaned up kill_all_lockers() 1. zend_accel_error was only executing clean up if log_verbosity_level is high enough to log 2. Cleaned up kill_all_lockers function and fixed comments. --- diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index f756f37a48..38a95413cd 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -619,27 +619,42 @@ static void accel_use_shm_interned_strings(void) #ifndef ZEND_WIN32 static inline void kill_all_lockers(struct flock *mem_usage_check) { - int tries = 10; - + int success, tries; /* so that other process won't try to force while we are busy cleaning up */ ZCSG(force_restart_time) = 0; while (mem_usage_check->l_pid > 0) { + /* Clear previous errno, reset success and tries */ + errno = 0; + success = 0; + tries = 10; + while (tries--) { - zend_accel_error(ACCEL_LOG_WARNING, "Killed locker %d", mem_usage_check->l_pid); + zend_accel_error(ACCEL_LOG_WARNING, "Attempting to kill locker %d", mem_usage_check->l_pid); if (kill(mem_usage_check->l_pid, SIGKILL)) { + if (errno == ESRCH) { + /* Process died before the signal was sent */ + success = 1; + zend_accel_error(ACCEL_LOG_WARNING, "Process %d died before SIGKILL was sent", mem_usage_check->l_pid); + } break; } /* give it a chance to die */ usleep(20000); if (kill(mem_usage_check->l_pid, 0)) { - /* can't kill it */ + if (errno == ESRCH) { + /* successfully killed locker, process no longer exists */ + success = 1; + zend_accel_error(ACCEL_LOG_WARNING, "Killed locker %d", mem_usage_check->l_pid); + } break; } usleep(10000); } - if (!tries) { - zend_accel_error(ACCEL_LOG_WARNING, "Can't kill %d after 10 tries!", mem_usage_check->l_pid); + if (!success) { + /* errno is not ESRCH or we ran out of tries to kill the locker */ ZCSG(force_restart_time) = time(NULL); /* restore forced restart request */ + /* cannot kill the locker, bail out with error */ + zend_accel_error(ACCEL_LOG_ERROR, "Cannot kill process %d: %s!", mem_usage_check->l_pid, strerror(errno)); } mem_usage_check->l_type = F_WRLCK; diff --git a/ext/opcache/tests/log_verbosity_bug.phpt b/ext/opcache/tests/log_verbosity_bug.phpt new file mode 100644 index 0000000000..725b8889f4 --- /dev/null +++ b/ext/opcache/tests/log_verbosity_bug.phpt @@ -0,0 +1,19 @@ +--TEST-- +Test ACCEL_LOG_FATAL will cause the process to die even if not logged +--DESCRIPTION-- +This test forces the opcache to error by setting memory_comsumption very large. +The resulting ACCEL_LOG_FATAL should cause php to die. +The process should die regardless of the log_verbosity_level. +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.memory_consumption=999999999 +opcache.log_verbosity_level=-1 +--SKIPIF-- + +--FILE-- + +--EXPECTF-- + diff --git a/ext/opcache/zend_accelerator_debug.c b/ext/opcache/zend_accelerator_debug.c index 8fcc01b4d4..12f80ab79b 100644 --- a/ext/opcache/zend_accelerator_debug.c +++ b/ext/opcache/zend_accelerator_debug.c @@ -30,22 +30,20 @@ void zend_accel_error(int type, const char *format, ...) { - va_list args; + va_list args; time_t timestamp; char *time_string; FILE * fLog = NULL; - if (type > ZCG(accel_directives).log_verbosity_level) { - return; - } + if (type <= ZCG(accel_directives).log_verbosity_level) { timestamp = time(NULL); time_string = asctime(localtime(×tamp)); time_string[24] = 0; if (!ZCG(accel_directives).error_log || - !*ZCG(accel_directives).error_log || - strcmp(ZCG(accel_directives).error_log, "stderr") == 0) { + !*ZCG(accel_directives).error_log || + strcmp(ZCG(accel_directives).error_log, "stderr") == 0) { fLog = stderr; } else { @@ -56,33 +54,40 @@ void zend_accel_error(int type, const char *format, ...) } #ifdef ZTS - fprintf(fLog, "%s (" ZEND_ULONG_FMT "): ", time_string, (zend_ulong)tsrm_thread_id()); + fprintf(fLog, "%s (" ZEND_ULONG_FMT "): ", time_string, (zend_ulong)tsrm_thread_id()); #else - fprintf(fLog, "%s (%d): ", time_string, getpid()); + fprintf(fLog, "%s (%d): ", time_string, getpid()); #endif - switch (type) { - case ACCEL_LOG_FATAL: - fprintf(fLog, "Fatal Error "); - break; - case ACCEL_LOG_ERROR: - fprintf(fLog, "Error "); - break; - case ACCEL_LOG_WARNING: - fprintf(fLog, "Warning "); - break; - case ACCEL_LOG_INFO: - fprintf(fLog, "Message "); - break; - case ACCEL_LOG_DEBUG: - fprintf(fLog, "Debug "); - break; - } + switch (type) { + case ACCEL_LOG_FATAL: + fprintf(fLog, "Fatal Error "); + break; + case ACCEL_LOG_ERROR: + fprintf(fLog, "Error "); + break; + case ACCEL_LOG_WARNING: + fprintf(fLog, "Warning "); + break; + case ACCEL_LOG_INFO: + fprintf(fLog, "Message "); + break; + case ACCEL_LOG_DEBUG: + fprintf(fLog, "Debug "); + break; + } + + va_start(args, format); + vfprintf(fLog, format, args); + va_end(args); + fprintf(fLog, "\n"); - va_start(args, format); - vfprintf(fLog, format, args); - va_end(args); - fprintf(fLog, "\n"); + fflush(fLog); + if (fLog != stderr) { + fclose(fLog); + } + } + /* perform error handling even without logging the error */ switch (type) { case ACCEL_LOG_ERROR: zend_bailout(); @@ -91,8 +96,5 @@ void zend_accel_error(int type, const char *format, ...) exit(-2); break; } - fflush(fLog); - if (fLog != stderr) { - fclose(fLog); - } + }