Improved shared interned strings handling. The previous implementatio…

…n 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.
php · Oct 25, 2018 · 33e777a · 33e777a
1 parent f33da6f
commit 33e777a
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 50 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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 @@ static int accel_post_startup(void)
 
 	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
@@ -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