From 38f1288b6427fc9e2fa2b5ad9912745ded923ee7 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 23 Jul 2019 10:38:23 +0200 Subject: [PATCH] Fix Zend signals unblocking There are a few parts here: * opcache should not be blocking signals while invoking compile_file, otherwise signals may remain blocked on a compile error. While at it, also protect SHM memory during compile_file. * We should deactivate Zend signals at the end of the request, to make sure that we gracefully recover from a missing unblock and signals don't remain blocked forever. * We don't use a critical section in deactivation, because it should not be necessary. Additionally we want to clean up the signal queue, if it is non-empty. * Enable SIGG(check) in debug builds so we notice issues in the future. --- Zend/zend_signal.c | 21 ++++++++++++++++----- ext/opcache/ZendAccelerator.c | 11 ++++++++++- main/main.c | 5 +++++ 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/Zend/zend_signal.c b/Zend/zend_signal.c index 1cfab63eee..f43de50b64 100644 --- a/Zend/zend_signal.c +++ b/Zend/zend_signal.c @@ -330,13 +330,13 @@ void zend_signal_activate(void) SIGG(active) = 1; SIGG(depth) = 0; + SIGG(check) = ZEND_DEBUG; } /* }}} */ /* {{{ zend_signal_deactivate * */ void zend_signal_deactivate(void) { - if (SIGG(check)) { size_t x; struct sigaction sa; @@ -344,21 +344,32 @@ void zend_signal_deactivate(void) if (SIGG(depth) != 0) { zend_error(E_CORE_WARNING, "zend_signal: shutdown with non-zero blocking depth (%d)", SIGG(depth)); } + /* did anyone steal our installed handler */ for (x = 0; x < sizeof(zend_sigs) / sizeof(*zend_sigs); x++) { sigaction(zend_sigs[x], NULL, &sa); - if (sa.sa_sigaction != zend_signal_handler_defer) { + if (sa.sa_sigaction != zend_signal_handler_defer && + sa.sa_sigaction != (void *) SIG_IGN) { zend_error(E_CORE_WARNING, "zend_signal: handler was replaced for signal (%d) after startup", zend_sigs[x]); } } } - SIGNAL_BEGIN_CRITICAL(); - SIGG(active) = 0; + /* After active=0 is set, signal handlers will be called directly and other + * state that is reset below will not be accessed. */ + *((volatile int *) &SIGG(active)) = 0; + SIGG(running) = 0; SIGG(blocked) = 0; SIGG(depth) = 0; - SIGNAL_END_CRITICAL(); + + /* If there are any queued signals because of a missed unblock, drop them. */ + if (SIGG(phead) && SIGG(ptail)) { + SIGG(ptail)->next = SIGG(pavail); + SIGG(pavail) = SIGG(phead); + SIGG(phead) = NULL; + SIGG(ptail) = NULL; + } } /* }}} */ diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index 1562922424..58016ce191 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -1570,7 +1570,9 @@ static zend_persistent_script *opcache_compile_file(zend_file_handle *file_handl /* check blacklist right after ensuring that file was opened */ if (file_handle->opened_path && zend_accel_blacklist_is_blacklisted(&accel_blacklist, ZSTR_VAL(file_handle->opened_path))) { + SHM_UNPROTECT(); ZCSG(blacklist_misses)++; + SHM_PROTECT(); *op_array_p = accelerator_orig_compile_file(file_handle, type); return NULL; } @@ -1601,7 +1603,9 @@ static zend_persistent_script *opcache_compile_file(zend_file_handle *file_handl } if (ZCG(accel_directives).max_file_size > 0 && size > (size_t)ZCG(accel_directives).max_file_size) { + SHM_UNPROTECT(); ZCSG(blacklist_misses)++; + SHM_PROTECT(); *op_array_p = accelerator_orig_compile_file(file_handle, type); return NULL; } @@ -2003,11 +2007,16 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type) return accelerator_orig_compile_file(file_handle, type); } + SHM_PROTECT(); + HANDLE_UNBLOCK_INTERRUPTIONS(); + persistent_script = opcache_compile_file(file_handle, type, key, &op_array); + HANDLE_BLOCK_INTERRUPTIONS(); + SHM_UNPROTECT(); + /* Try and cache the script and assume that it is returned from_shared_memory. * If it isn't compile_and_cache_file() changes the flag to 0 */ from_shared_memory = 0; - persistent_script = opcache_compile_file(file_handle, type, key, &op_array); if (persistent_script) { persistent_script = cache_script_in_shared_memory(persistent_script, key, key ? key_length : 0, &from_shared_memory); } diff --git a/main/main.c b/main/main.c index d68cb6eca4..37d520ec34 100644 --- a/main/main.c +++ b/main/main.c @@ -1932,6 +1932,11 @@ void php_request_shutdown(void *dummy) zend_unset_timeout(); } zend_end_try(); + /* 17. Deactivate Zend signals */ +#ifdef ZEND_SIGNALS + zend_signal_deactivate(); +#endif + #ifdef PHP_WIN32 if (PG(com_initialized)) { CoUninitialize(); -- 2.40.0