From 33e777acbfb9bbd2a7efabc7365104780db4c6b0 Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Thu, 25 Oct 2018 20:30:51 +0300 Subject: [PATCH] Improved shared interned strings handling. The previous implementation worked incorrectly in ZTS build. It changed strings only in function/class tables of one thread. Now all threads gets the same shared interned strings. Also, on shutdown, we don't try to replace SHM interned strings back to process strings, but delay dettachment of SHM instead. --- Zend/zend.c | 23 +++++++++++-------- Zend/zend.h | 3 +++ Zend/zend_string.c | 16 ------------- Zend/zend_string.h | 1 - ext/opcache/ZendAccelerator.c | 42 +++++++++++++++-------------------- main/main.c | 7 ++++++ 6 files changed, 42 insertions(+), 50 deletions(-) diff --git a/Zend/zend.c b/Zend/zend.c index 1e4d2a0f421..61ac0c62490 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -78,6 +78,7 @@ void (*zend_printf_to_smart_str)(smart_str *buf, const char *format, va_list ap) ZEND_API char *(*zend_getenv)(char *name, size_t name_len); ZEND_API zend_string *(*zend_resolve_path)(const char *filename, size_t filename_len); ZEND_API int (*zend_post_startup_cb)(void) = NULL; +ZEND_API void (*zend_post_shutdown_cb)(void) = NULL; void (*zend_on_timeout)(int seconds); @@ -954,7 +955,18 @@ int zend_post_startup(void) /* {{{ */ zend_compiler_globals *compiler_globals = ts_resource(compiler_globals_id); zend_executor_globals *executor_globals = ts_resource(executor_globals_id); +#endif + if (zend_post_startup_cb) { + int (*cb)(void) = zend_post_startup_cb; + + zend_post_startup_cb = NULL; + if (cb() != SUCCESS) { + return FAILURE; + } + } + +#ifdef ZTS *GLOBAL_FUNCTION_TABLE = *compiler_globals->function_table; *GLOBAL_CLASS_TABLE = *compiler_globals->class_table; *GLOBAL_CONSTANTS_TABLE = *executor_globals->zend_constants; @@ -981,15 +993,6 @@ int zend_post_startup(void) /* {{{ */ global_map_ptr_last = CG(map_ptr_last); #endif - if (zend_post_startup_cb) { - int (*cb)(void) = zend_post_startup_cb; - - zend_post_startup_cb = NULL; - if (cb() != SUCCESS) { - return FAILURE; - } - } - return SUCCESS; } /* }}} */ @@ -1025,6 +1028,8 @@ void zend_shutdown(void) /* {{{ */ GLOBAL_CLASS_TABLE = NULL; GLOBAL_AUTO_GLOBALS_TABLE = NULL; GLOBAL_CONSTANTS_TABLE = NULL; + ts_free_id(executor_globals_id); + ts_free_id(compiler_globals_id); #else if (CG(map_ptr_base)) { free(CG(map_ptr_base)); diff --git a/Zend/zend.h b/Zend/zend.h index 0d3ce9ffa8e..c7885a68a5f 100644 --- a/Zend/zend.h +++ b/Zend/zend.h @@ -293,7 +293,10 @@ extern void (*zend_printf_to_smart_string)(smart_string *buf, const char *format extern void (*zend_printf_to_smart_str)(smart_str *buf, const char *format, va_list ap); extern ZEND_API char *(*zend_getenv)(char *name, size_t name_len); extern ZEND_API zend_string *(*zend_resolve_path)(const char *filename, size_t filename_len); + +/* These two callbacks are especially for opcache */ extern ZEND_API int (*zend_post_startup_cb)(void); +extern ZEND_API void (*zend_post_shutdown_cb)(void); ZEND_API ZEND_COLD void zend_error(int type, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3); ZEND_API ZEND_COLD void zend_throw_error(zend_class_entry *exception_ce, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3); diff --git a/Zend/zend_string.c b/Zend/zend_string.c index b047202a14d..1ef6a17c0c1 100644 --- a/Zend/zend_string.c +++ b/Zend/zend_string.c @@ -39,8 +39,6 @@ static HashTable interned_strings_permanent; static zend_new_interned_string_func_t interned_string_request_handler = zend_new_interned_string_request; static zend_string_init_interned_func_t interned_string_init_request_handler = zend_string_init_interned_request; -static zend_string_copy_storage_func_t interned_string_copy_storage = NULL; -static zend_string_copy_storage_func_t interned_string_restore_storage = NULL; ZEND_API zend_string *zend_empty_string = NULL; ZEND_API zend_string *zend_one_char_string[256]; @@ -83,8 +81,6 @@ ZEND_API void zend_interned_strings_init(void) interned_string_request_handler = zend_new_interned_string_request; interned_string_init_request_handler = zend_string_init_interned_request; - interned_string_copy_storage = NULL; - interned_string_restore_storage = NULL; zend_empty_string = NULL; zend_known_strings = NULL; @@ -301,26 +297,14 @@ ZEND_API void zend_interned_strings_set_request_storage_handlers(zend_new_intern interned_string_init_request_handler = init_handler; } -ZEND_API void zend_interned_strings_set_permanent_storage_copy_handlers(zend_string_copy_storage_func_t copy_handler, zend_string_copy_storage_func_t restore_handler) -{ - interned_string_copy_storage = copy_handler; - interned_string_restore_storage = restore_handler; -} - ZEND_API void zend_interned_strings_switch_storage(zend_bool request) { if (request) { - if (interned_string_copy_storage) { - interned_string_copy_storage(); - } zend_new_interned_string = interned_string_request_handler; zend_string_init_interned = interned_string_init_request_handler; } else { zend_new_interned_string = zend_new_interned_string_permanent; zend_string_init_interned = zend_string_init_interned_permanent; - if (interned_string_restore_storage) { - interned_string_restore_storage(); - } } } diff --git a/Zend/zend_string.h b/Zend/zend_string.h index d34cf5862d1..1ee4e6d135f 100644 --- a/Zend/zend_string.h +++ b/Zend/zend_string.h @@ -39,7 +39,6 @@ ZEND_API void zend_interned_strings_dtor(void); ZEND_API void zend_interned_strings_activate(void); ZEND_API void zend_interned_strings_deactivate(void); ZEND_API void zend_interned_strings_set_request_storage_handlers(zend_new_interned_string_func_t handler, zend_string_init_interned_func_t init_handler); -ZEND_API void zend_interned_strings_set_permanent_storage_copy_handlers(zend_string_copy_storage_func_t copy_handler, zend_string_copy_storage_func_t restore_handler); ZEND_API void zend_interned_strings_switch_storage(zend_bool request); ZEND_API extern zend_string *zend_empty_string; diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index b834d81679c..88dd6fe7d45 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -726,19 +726,6 @@ static zend_string* ZEND_FASTCALL accel_replace_string_by_shm_permanent(zend_str return str; } -static zend_string* ZEND_FASTCALL accel_replace_string_by_process_permanent(zend_string *str) -{ - zend_string *ret = zend_interned_string_find_permanent(str); - - if (ret) { - zend_string_release(str); - return ret; - } - ZEND_ASSERT(0); - return str; -} - - static void accel_use_shm_interned_strings(void) { HANDLE_BLOCK_INTERRUPTIONS(); @@ -760,11 +747,6 @@ static void accel_use_shm_interned_strings(void) HANDLE_UNBLOCK_INTERRUPTIONS(); } -static void accel_use_permanent_interned_strings(void) -{ - accel_copy_permanent_strings(accel_replace_string_by_process_permanent); -} - #ifndef ZEND_WIN32 static inline void kill_all_lockers(struct flock *mem_usage_check) { @@ -2553,8 +2535,6 @@ static int zend_accel_init_shm(void) STRTAB_INVALID_POS, (char*)ZCSG(interned_strings).start - ((char*)&ZCSG(interned_strings) + sizeof(zend_string_table))); - - zend_interned_strings_set_permanent_storage_copy_handlers(accel_use_shm_interned_strings, accel_use_permanent_interned_strings); } zend_interned_strings_set_request_storage_handlers(accel_new_interned_string_for_php, accel_init_interned_string_for_php); @@ -2777,6 +2757,9 @@ static int accel_startup(zend_extension *extension) orig_post_startup_cb = zend_post_startup_cb; zend_post_startup_cb = accel_post_startup; + /* Prevent unloadig */ + extension->handle = 0; + return SUCCESS; } @@ -2817,9 +2800,6 @@ static int accel_post_startup(void) case SUCCESSFULLY_REATTACHED: zend_shared_alloc_lock(); accel_shared_globals = (zend_accel_shared_globals *) ZSMMG(app_shared_globals); - if (ZCG(accel_directives).interned_strings_buffer) { - zend_interned_strings_set_permanent_storage_copy_handlers(accel_use_shm_interned_strings, accel_use_permanent_interned_strings); - } zend_interned_strings_set_request_storage_handlers(accel_new_interned_string_for_php, accel_init_interned_string_for_php); zend_shared_alloc_unlock(); break; @@ -2915,9 +2895,20 @@ file_cache_fallback: zend_optimizer_startup(); + if (!file_cache_only && ZCG(accel_directives).interned_strings_buffer) { + accel_use_shm_interned_strings(); + } + return SUCCESS; } +static void (*orig_post_shutdown_cb)(void); + +static void accel_post_shutdown(void) +{ + zend_shared_alloc_shutdown(); +} + void accel_shutdown(void) { zend_ini_entry *ini_entry; @@ -2945,8 +2936,11 @@ void accel_shutdown(void) #endif if (!_file_cache_only) { - zend_shared_alloc_shutdown(); + /* Delay SHM dettach */ + orig_post_shutdown_cb = zend_post_shutdown_cb; + zend_post_shutdown_cb = accel_post_shutdown; } + zend_compile_file = accelerator_orig_compile_file; if ((ini_entry = zend_hash_str_find_ptr(EG(ini_directives), "include_path", sizeof("include_path")-1)) != NULL) { diff --git a/main/main.c b/main/main.c index e77ec61ca87..290dba00fe2 100644 --- a/main/main.c +++ b/main/main.c @@ -2520,6 +2520,13 @@ void php_module_shutdown(void) zend_interned_strings_dtor(); #endif + if (zend_post_shutdown_cb) { + void (*cb)(void) = zend_post_shutdown_cb; + + zend_post_shutdown_cb = NULL; + cb(); + } + module_initialized = 0; #ifndef ZTS