Skip to content

Commit 33e777a

Browse files
dstogovdstogov
committed
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.
1 parent f33da6f commit 33e777a

File tree

6 files changed

+42
-50
lines changed

6 files changed

+42
-50
lines changed

‎Zend/zend.c‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ void (*zend_printf_to_smart_str)(smart_str *buf, const char *format, va_list ap)
7878
ZEND_API char *(*zend_getenv)(char *name, size_t name_len);
7979
ZEND_API zend_string *(*zend_resolve_path)(const char *filename, size_t filename_len);
8080
ZEND_API int (*zend_post_startup_cb)(void) = NULL;
81+
ZEND_API void (*zend_post_shutdown_cb)(void) = NULL;
8182

8283
void (*zend_on_timeout)(int seconds);
8384

@@ -954,7 +955,18 @@ int zend_post_startup(void) /* {{{ */
954955

955956
zend_compiler_globals *compiler_globals = ts_resource(compiler_globals_id);
956957
zend_executor_globals *executor_globals = ts_resource(executor_globals_id);
958+
#endif
959+
960+
if (zend_post_startup_cb) {
961+
int (*cb)(void) = zend_post_startup_cb;
962+
963+
zend_post_startup_cb = NULL;
964+
if (cb() != SUCCESS) {
965+
return FAILURE;
966+
}
967+
}
957968

969+
#ifdef ZTS
958970
*GLOBAL_FUNCTION_TABLE = *compiler_globals->function_table;
959971
*GLOBAL_CLASS_TABLE = *compiler_globals->class_table;
960972
*GLOBAL_CONSTANTS_TABLE = *executor_globals->zend_constants;
@@ -981,15 +993,6 @@ int zend_post_startup(void) /* {{{ */
981993
global_map_ptr_last = CG(map_ptr_last);
982994
#endif
983995

984-
if (zend_post_startup_cb) {
985-
int (*cb)(void) = zend_post_startup_cb;
986-
987-
zend_post_startup_cb = NULL;
988-
if (cb() != SUCCESS) {
989-
return FAILURE;
990-
}
991-
}
992-
993996
return SUCCESS;
994997
}
995998
/* }}} */
@@ -1025,6 +1028,8 @@ void zend_shutdown(void) /* {{{ */
10251028
GLOBAL_CLASS_TABLE = NULL;
10261029
GLOBAL_AUTO_GLOBALS_TABLE = NULL;
10271030
GLOBAL_CONSTANTS_TABLE = NULL;
1031+
ts_free_id(executor_globals_id);
1032+
ts_free_id(compiler_globals_id);
10281033
#else
10291034
if (CG(map_ptr_base)) {
10301035
free(CG(map_ptr_base));

‎Zend/zend.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,10 @@ extern void (*zend_printf_to_smart_string)(smart_string *buf, const char *format
293293
extern void (*zend_printf_to_smart_str)(smart_str *buf, const char *format, va_list ap);
294294
extern ZEND_API char *(*zend_getenv)(char *name, size_t name_len);
295295
extern ZEND_API zend_string *(*zend_resolve_path)(const char *filename, size_t filename_len);
296+
297+
/* These two callbacks are especially for opcache */
296298
extern ZEND_API int (*zend_post_startup_cb)(void);
299+
extern ZEND_API void (*zend_post_shutdown_cb)(void);
297300

298301
ZEND_API ZEND_COLD void zend_error(int type, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3);
299302
ZEND_API ZEND_COLD void zend_throw_error(zend_class_entry *exception_ce, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3);

‎Zend/zend_string.c‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ static HashTable interned_strings_permanent;
3939

4040
static zend_new_interned_string_func_t interned_string_request_handler = zend_new_interned_string_request;
4141
static zend_string_init_interned_func_t interned_string_init_request_handler = zend_string_init_interned_request;
42-
static zend_string_copy_storage_func_t interned_string_copy_storage = NULL;
43-
static zend_string_copy_storage_func_t interned_string_restore_storage = NULL;
4442

4543
ZEND_API zend_string *zend_empty_string = NULL;
4644
ZEND_API zend_string *zend_one_char_string[256];
@@ -83,8 +81,6 @@ ZEND_API void zend_interned_strings_init(void)
8381

8482
interned_string_request_handler = zend_new_interned_string_request;
8583
interned_string_init_request_handler = zend_string_init_interned_request;
86-
interned_string_copy_storage = NULL;
87-
interned_string_restore_storage = NULL;
8884

8985
zend_empty_string = NULL;
9086
zend_known_strings = NULL;
@@ -301,26 +297,14 @@ ZEND_API void zend_interned_strings_set_request_storage_handlers(zend_new_intern
301297
interned_string_init_request_handler = init_handler;
302298
}
303299

304-
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)
305-
{
306-
interned_string_copy_storage = copy_handler;
307-
interned_string_restore_storage = restore_handler;
308-
}
309-
310300
ZEND_API void zend_interned_strings_switch_storage(zend_bool request)
311301
{
312302
if (request) {
313-
if (interned_string_copy_storage) {
314-
interned_string_copy_storage();
315-
}
316303
zend_new_interned_string = interned_string_request_handler;
317304
zend_string_init_interned = interned_string_init_request_handler;
318305
} else {
319306
zend_new_interned_string = zend_new_interned_string_permanent;
320307
zend_string_init_interned = zend_string_init_interned_permanent;
321-
if (interned_string_restore_storage) {
322-
interned_string_restore_storage();
323-
}
324308
}
325309
}
326310

‎Zend/zend_string.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ ZEND_API void zend_interned_strings_dtor(void);
3939
ZEND_API void zend_interned_strings_activate(void);
4040
ZEND_API void zend_interned_strings_deactivate(void);
4141
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);
42-
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);
4342
ZEND_API void zend_interned_strings_switch_storage(zend_bool request);
4443

4544
ZEND_API extern zend_string *zend_empty_string;

‎ext/opcache/ZendAccelerator.c‎

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -726,19 +726,6 @@ static zend_string* ZEND_FASTCALL accel_replace_string_by_shm_permanent(zend_str
726726
return str;
727727
}
728728

729-
static zend_string* ZEND_FASTCALL accel_replace_string_by_process_permanent(zend_string *str)
730-
{
731-
zend_string *ret = zend_interned_string_find_permanent(str);
732-
733-
if (ret) {
734-
zend_string_release(str);
735-
return ret;
736-
}
737-
ZEND_ASSERT(0);
738-
return str;
739-
}
740-
741-
742729
static void accel_use_shm_interned_strings(void)
743730
{
744731
HANDLE_BLOCK_INTERRUPTIONS();
@@ -760,11 +747,6 @@ static void accel_use_shm_interned_strings(void)
760747
HANDLE_UNBLOCK_INTERRUPTIONS();
761748
}
762749

763-
static void accel_use_permanent_interned_strings(void)
764-
{
765-
accel_copy_permanent_strings(accel_replace_string_by_process_permanent);
766-
}
767-
768750
#ifndef ZEND_WIN32
769751
static inline void kill_all_lockers(struct flock *mem_usage_check)
770752
{
@@ -2553,8 +2535,6 @@ static int zend_accel_init_shm(void)
25532535
STRTAB_INVALID_POS,
25542536
(char*)ZCSG(interned_strings).start -
25552537
((char*)&ZCSG(interned_strings) + sizeof(zend_string_table)));
2556-
2557-
zend_interned_strings_set_permanent_storage_copy_handlers(accel_use_shm_interned_strings, accel_use_permanent_interned_strings);
25582538
}
25592539

25602540
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)
27772757
orig_post_startup_cb = zend_post_startup_cb;
27782758
zend_post_startup_cb = accel_post_startup;
27792759

2760+
/* Prevent unloadig */
2761+
extension->handle = 0;
2762+
27802763
return SUCCESS;
27812764
}
27822765

@@ -2817,9 +2800,6 @@ static int accel_post_startup(void)
28172800
case SUCCESSFULLY_REATTACHED:
28182801
zend_shared_alloc_lock();
28192802
accel_shared_globals = (zend_accel_shared_globals *) ZSMMG(app_shared_globals);
2820-
if (ZCG(accel_directives).interned_strings_buffer) {
2821-
zend_interned_strings_set_permanent_storage_copy_handlers(accel_use_shm_interned_strings, accel_use_permanent_interned_strings);
2822-
}
28232803
zend_interned_strings_set_request_storage_handlers(accel_new_interned_string_for_php, accel_init_interned_string_for_php);
28242804
zend_shared_alloc_unlock();
28252805
break;
@@ -2915,9 +2895,20 @@ static int accel_post_startup(void)
29152895

29162896
zend_optimizer_startup();
29172897

2898+
if (!file_cache_only && ZCG(accel_directives).interned_strings_buffer) {
2899+
accel_use_shm_interned_strings();
2900+
}
2901+
29182902
return SUCCESS;
29192903
}
29202904

2905+
static void (*orig_post_shutdown_cb)(void);
2906+
2907+
static void accel_post_shutdown(void)
2908+
{
2909+
zend_shared_alloc_shutdown();
2910+
}
2911+
29212912
void accel_shutdown(void)
29222913
{
29232914
zend_ini_entry *ini_entry;
@@ -2945,8 +2936,11 @@ void accel_shutdown(void)
29452936
#endif
29462937

29472938
if (!_file_cache_only) {
2948-
zend_shared_alloc_shutdown();
2939+
/* Delay SHM dettach */
2940+
orig_post_shutdown_cb = zend_post_shutdown_cb;
2941+
zend_post_shutdown_cb = accel_post_shutdown;
29492942
}
2943+
29502944
zend_compile_file = accelerator_orig_compile_file;
29512945

29522946
if ((ini_entry = zend_hash_str_find_ptr(EG(ini_directives), "include_path", sizeof("include_path")-1)) != NULL) {

‎main/main.c‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2520,6 +2520,13 @@ void php_module_shutdown(void)
25202520
zend_interned_strings_dtor();
25212521
#endif
25222522

2523+
if (zend_post_shutdown_cb) {
2524+
void (*cb)(void) = zend_post_shutdown_cb;
2525+
2526+
zend_post_shutdown_cb = NULL;
2527+
cb();
2528+
}
2529+
25232530
module_initialized = 0;
25242531

25252532
#ifndef ZTS

0 commit comments

Comments
 (0)