From ecea7d3d35035522bb27c9db75c4f87433108ddb Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Wed, 24 Feb 2016 21:15:53 +0300 Subject: [PATCH] Fixed more synchronisation issues during SHM reload --- ext/opcache/ZendAccelerator.c | 218 +++++++++++++++++--------------- ext/opcache/zend_file_cache.c | 28 ++-- ext/opcache/zend_persist_calc.c | 11 +- 3 files changed, 141 insertions(+), 116 deletions(-) diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index addfe87ab5..cd91dfbc4b 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -220,6 +220,104 @@ static ZEND_INI_MH(accel_include_path_on_modify) return ret; } +static inline void accel_restart_enter(void) +{ +#ifdef ZEND_WIN32 + INCREMENT(restart_in); +#else + static const FLOCK_STRUCTURE(restart_in_progress, F_WRLCK, SEEK_SET, 2, 1); + + if (fcntl(lock_file, F_SETLK, &restart_in_progress) == -1) { + zend_accel_error(ACCEL_LOG_DEBUG, "RestartC(+1): %s (%d)", strerror(errno), errno); + } +#endif + ZCSG(restart_in_progress) = 1; +} + +static inline void accel_restart_leave(void) +{ +#ifdef ZEND_WIN32 + ZCSG(restart_in_progress) = 0; + DECREMENT(restart_in); +#else + static const FLOCK_STRUCTURE(restart_finished, F_UNLCK, SEEK_SET, 2, 1); + + ZCSG(restart_in_progress) = 0; + if (fcntl(lock_file, F_SETLK, &restart_finished) == -1) { + zend_accel_error(ACCEL_LOG_DEBUG, "RestartC(-1): %s (%d)", strerror(errno), errno); + } +#endif +} + +static inline int accel_restart_is_active(void) +{ + if (ZCSG(restart_in_progress)) { +#ifndef ZEND_WIN32 + FLOCK_STRUCTURE(restart_check, F_WRLCK, SEEK_SET, 2, 1); + + if (fcntl(lock_file, F_GETLK, &restart_check) == -1) { + zend_accel_error(ACCEL_LOG_DEBUG, "RestartC: %s (%d)", strerror(errno), errno); + return FAILURE; + } + if (restart_check.l_type == F_UNLCK) { + ZCSG(restart_in_progress) = 0; + return 0; + } else { + return 1; + } +#else + return LOCKVAL(restart_in) != 0; +#endif + } + return 0; +} + +/* Creates a read lock for SHM access */ +static inline int accel_activate_add(void) +{ +#ifdef ZEND_WIN32 + INCREMENT(mem_usage); +#else + static const FLOCK_STRUCTURE(mem_usage_lock, F_RDLCK, SEEK_SET, 1, 1); + + if (fcntl(lock_file, F_SETLK, &mem_usage_lock) == -1) { + zend_accel_error(ACCEL_LOG_DEBUG, "UpdateC(+1): %s (%d)", strerror(errno), errno); + return FAILURE; + } +#endif + return SUCCESS; +} + +/* Releases a lock for SHM access */ +static inline void accel_deactivate_sub(void) +{ +#ifdef ZEND_WIN32 + if (ZCG(counted)) { + DECREMENT(mem_usage); + ZCG(counted) = 0; + } +#else + static const FLOCK_STRUCTURE(mem_usage_unlock, F_UNLCK, SEEK_SET, 1, 1); + + if (fcntl(lock_file, F_SETLK, &mem_usage_unlock) == -1) { + zend_accel_error(ACCEL_LOG_DEBUG, "UpdateC(-1): %s (%d)", strerror(errno), errno); + } +#endif +} + +static inline void accel_unlock_all(void) +{ +#ifdef ZEND_WIN32 + accel_deactivate_sub(); +#else + static const FLOCK_STRUCTURE(mem_usage_unlock_all, F_UNLCK, SEEK_SET, 0, 0); + + if (fcntl(lock_file, F_SETLK, &mem_usage_unlock_all) == -1) { + zend_accel_error(ACCEL_LOG_DEBUG, "UnlockAll: %s (%d)", strerror(errno), errno); + } +#endif +} + /* Interned strings support */ static zend_string *(*orig_new_interned_string)(zend_string *str); static void (*orig_interned_strings_snapshot)(void); @@ -291,6 +389,12 @@ static zend_string *accel_find_interned_string(zend_string *str) /* this is already an interned string */ return str; } + if (!ZCG(counted)) { + if (accel_activate_add() == FAILURE) { + return str; + } + ZCG(counted) = 1; + } h = zend_string_hash_val(str); nIndex = h | ZCSG(interned_strings).nTableMask; @@ -492,102 +596,6 @@ static void accel_use_shm_interned_strings(void) } #endif -static inline void accel_restart_enter(void) -{ -#ifdef ZEND_WIN32 - INCREMENT(restart_in); -#else - static const FLOCK_STRUCTURE(restart_in_progress, F_WRLCK, SEEK_SET, 2, 1); - - if (fcntl(lock_file, F_SETLK, &restart_in_progress) == -1) { - zend_accel_error(ACCEL_LOG_DEBUG, "RestartC(+1): %s (%d)", strerror(errno), errno); - } -#endif - ZCSG(restart_in_progress) = 1; -} - -static inline void accel_restart_leave(void) -{ -#ifdef ZEND_WIN32 - ZCSG(restart_in_progress) = 0; - DECREMENT(restart_in); -#else - static const FLOCK_STRUCTURE(restart_finished, F_UNLCK, SEEK_SET, 2, 1); - - ZCSG(restart_in_progress) = 0; - if (fcntl(lock_file, F_SETLK, &restart_finished) == -1) { - zend_accel_error(ACCEL_LOG_DEBUG, "RestartC(-1): %s (%d)", strerror(errno), errno); - } -#endif -} - -static inline int accel_restart_is_active(void) -{ - if (ZCSG(restart_in_progress)) { -#ifndef ZEND_WIN32 - FLOCK_STRUCTURE(restart_check, F_WRLCK, SEEK_SET, 2, 1); - - if (fcntl(lock_file, F_GETLK, &restart_check) == -1) { - zend_accel_error(ACCEL_LOG_DEBUG, "RestartC: %s (%d)", strerror(errno), errno); - return FAILURE; - } - if (restart_check.l_type == F_UNLCK) { - ZCSG(restart_in_progress) = 0; - return 0; - } else { - return 1; - } -#else - return LOCKVAL(restart_in) != 0; -#endif - } - return 0; -} - -/* Creates a read lock for SHM access */ -static inline void accel_activate_add(void) -{ -#ifdef ZEND_WIN32 - INCREMENT(mem_usage); -#else - static const FLOCK_STRUCTURE(mem_usage_lock, F_RDLCK, SEEK_SET, 1, 1); - - if (fcntl(lock_file, F_SETLK, &mem_usage_lock) == -1) { - zend_accel_error(ACCEL_LOG_DEBUG, "UpdateC(+1): %s (%d)", strerror(errno), errno); - } -#endif -} - -/* Releases a lock for SHM access */ -static inline void accel_deactivate_sub(void) -{ -#ifdef ZEND_WIN32 - if (ZCG(counted)) { - DECREMENT(mem_usage); - ZCG(counted) = 0; - } -#else - static const FLOCK_STRUCTURE(mem_usage_unlock, F_UNLCK, SEEK_SET, 1, 1); - - if (fcntl(lock_file, F_SETLK, &mem_usage_unlock) == -1) { - zend_accel_error(ACCEL_LOG_DEBUG, "UpdateC(-1): %s (%d)", strerror(errno), errno); - } -#endif -} - -static inline void accel_unlock_all(void) -{ -#ifdef ZEND_WIN32 - accel_deactivate_sub(); -#else - static const FLOCK_STRUCTURE(mem_usage_unlock_all, F_UNLCK, SEEK_SET, 0, 0); - - if (fcntl(lock_file, F_SETLK, &mem_usage_unlock_all) == -1) { - zend_accel_error(ACCEL_LOG_DEBUG, "UnlockAll: %s (%d)", strerror(errno), errno); - } -#endif -} - #ifndef ZEND_WIN32 static inline void kill_all_lockers(struct flock *mem_usage_check) { @@ -1563,7 +1571,9 @@ zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int type) } } + SHM_UNPROTECT(); persistent_script = zend_file_cache_script_load(file_handle); + SHM_PROTECT(); if (persistent_script) { /* see bug #15471 (old BTS) */ if (persistent_script->full_path) { @@ -1589,8 +1599,6 @@ zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int type) } zend_file_handle_dtor(file_handle); - persistent_script->dynamic_members.last_used = ZCG(request_time); - if (persistent_script->ping_auto_globals_mask) { zend_accel_set_auto_globals(persistent_script->ping_auto_globals_mask); } @@ -1709,8 +1717,15 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type) * each execution) */ if (!ZCG(counted)) { + if (accel_activate_add() == FAILURE) { +#ifdef HAVE_OPCACHE_FILE_CACHE + if (ZCG(accel_directives).file_cache) { + return file_cache_compile_file(file_handle, type); + } +#endif + return accelerator_orig_compile_file(file_handle, type); + } ZCG(counted) = 1; - accel_activate_add(); } SHM_UNPROTECT(); @@ -2874,13 +2889,16 @@ int accelerator_shm_read_lock(void) } else { /* here accelerator is active but we do not hold SHM lock. This means restart was scheduled or is in progress now */ - accel_activate_add(); /* acquire usage lock */ + if (accel_activate_add() == FAILURE) { /* acquire usage lock */ + return FAILURE; + } /* Now if we weren't inside restart, restart would not begin until we remove usage lock */ if (ZCSG(restart_in_progress)) { /* we already were inside restart this means it's not safe to touch shm */ accel_deactivate_now(); /* drop usage lock */ return FAILURE; } + ZCG(counted) = 1; } return SUCCESS; } diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index 410921f800..b65abca225 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -132,13 +132,8 @@ static int zend_file_cache_flock(int fd, int type) } else { \ ZEND_ASSERT(IS_SERIALIZED(ptr)); \ (ptr) = (void*)((char*)buf + (size_t)(ptr)); \ - /* script->corrupted shows if the script in SHM or not */ \ - if (EXPECTED(!script->corrupted)) { \ - GC_FLAGS(ptr) |= IS_STR_INTERNED | IS_STR_PERMANENT; \ - } else { \ - GC_FLAGS(ptr) |= IS_STR_INTERNED; \ - GC_FLAGS(ptr) &= ~IS_STR_PERMANENT; \ - } \ + GC_FLAGS(ptr) |= IS_STR_INTERNED; \ + GC_FLAGS(ptr) &= ~IS_STR_PERMANENT; \ } \ } \ } while (0) @@ -224,15 +219,17 @@ static void *zend_file_cache_unserialize_interned(zend_string *str, int in_shm) zend_string *ret; str = (zend_string*)((char*)ZCG(mem) + ((size_t)(str) & ~Z_UL(1))); - ret = accel_new_interned_string(str); - if (ret == str) { - /* String wasn't interned but we will use it as interned anyway */ - if (in_shm) { - GC_FLAGS(ret) |= IS_STR_INTERNED | IS_STR_PERMANENT; - } else { + if (in_shm) { + ret = accel_new_interned_string(str); + if (ret == str) { + /* String wasn't interned but we will use it as interned anyway */ GC_FLAGS(ret) |= IS_STR_INTERNED; GC_FLAGS(ret) &= ~IS_STR_PERMANENT; } + } else { + ret = str; + GC_FLAGS(ret) |= IS_STR_INTERNED; + GC_FLAGS(ret) &= ~IS_STR_PERMANENT; } return ret; } @@ -1303,7 +1300,9 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl return NULL; } - if (!ZCG(accel_directives).file_cache_only) { + if (!ZCG(accel_directives).file_cache_only && + !ZCSG(restart_in_progress) && + accelerator_shm_read_lock() == SUCCESS) { /* exclusive lock */ zend_shared_alloc_lock(); @@ -1357,6 +1356,7 @@ use_process_mem: if (cache_it) { script->dynamic_members.checksum = zend_accel_script_checksum(script); + script->dynamic_members.last_used = ZCG(request_time); zend_accel_hash_update(&ZCSG(hash), ZSTR_VAL(script->full_path), ZSTR_LEN(script->full_path), 0, script); diff --git a/ext/opcache/zend_persist_calc.c b/ext/opcache/zend_persist_calc.c index 6c721196a4..25ba69bee8 100644 --- a/ext/opcache/zend_persist_calc.c +++ b/ext/opcache/zend_persist_calc.c @@ -34,7 +34,9 @@ # define ADD_STRING(str) ADD_DUP_SIZE((str), _ZSTR_STRUCT_SIZE(ZSTR_LEN(str))) # define ADD_INTERNED_STRING(str, do_free) do { \ - if (!IS_ACCEL_INTERNED(str)) { \ + if (ZCG(current_persistent_script)->corrupted) { \ + ADD_STRING(str); \ + } else if (!IS_ACCEL_INTERNED(str)) { \ zend_string *tmp = accel_new_interned_string(str); \ if (tmp != (str)) { \ if (do_free) { \ @@ -126,7 +128,7 @@ static void zend_persist_zval_calc(zval *z) case IS_CONSTANT: flags = Z_GC_FLAGS_P(z) & ~ (IS_STR_PERSISTENT | IS_STR_INTERNED | IS_STR_PERMANENT); ADD_INTERNED_STRING(Z_STR_P(z), 0); - if (!Z_REFCOUNTED_P(z)) { + if (ZSTR_IS_INTERNED(Z_STR_P(z))) { Z_TYPE_FLAGS_P(z) &= ~ (IS_TYPE_REFCOUNTED | IS_TYPE_COPYABLE); } Z_GC_FLAGS_P(z) |= flags; @@ -385,11 +387,15 @@ uint zend_accel_script_persist_calc(zend_persistent_script *new_persistent_scrip new_persistent_script->size = 0; new_persistent_script->arena_mem = NULL; new_persistent_script->arena_size = 0; + new_persistent_script->corrupted = 0; ZCG(current_persistent_script) = new_persistent_script; ADD_DUP_SIZE(new_persistent_script, sizeof(zend_persistent_script)); if (key) { ADD_DUP_SIZE(key, key_length + 1); + } else { + /* script is not going to be saved in SHM */ + new_persistent_script->corrupted = 1; } ADD_STRING(new_persistent_script->full_path); @@ -408,6 +414,7 @@ uint zend_accel_script_persist_calc(zend_persistent_script *new_persistent_scrip #endif new_persistent_script->size += new_persistent_script->arena_size; + new_persistent_script->corrupted = 0; ZCG(current_persistent_script) = NULL; -- 2.40.0