From b3c61e1e1b309330a39c1f03f88d10cf4c1e1f3f Mon Sep 17 00:00:00 2001 From: "Tianfeng.Han" Date: Fri, 13 Dec 2024 17:41:25 +0800 Subject: [PATCH] Refactor the implementation of runtime hooks under ZTS mode, Fix thread safety issues. (#5617) * refactor runtime hook with ZTS * fix * optimize code * fix type * fix tests * fix tests [2] * fix * fix * fix 4 * fix tests * optimize code, add core tests --- core-tests/src/coroutine/system.cpp | 9 + core-tests/src/os/wait.cpp | 71 ++++--- ext-src/php_swoole.cc | 8 + ext-src/php_swoole_cxx.h | 51 +++-- ext-src/stubs/php_swoole_runtime.stub.php | 2 +- ext-src/stubs/php_swoole_runtime_arginfo.h | 3 +- ext-src/swoole_coroutine_system.cc | 58 ++---- ext-src/swoole_runtime.cc | 205 +++++++++++-------- include/swoole_coroutine.h | 1 + include/swoole_coroutine_system.h | 6 + src/coroutine/system.cc | 47 ++++- src/os/wait.cc | 24 ++- tests/swoole_coroutine/cancel/wait.phpt | 10 +- tests/swoole_runtime/base.phpt | 6 +- tests/swoole_runtime/file_hook/bug_4327.phpt | 30 +-- tests/swoole_thread/pipe.phpt | 2 - tests/swoole_thread/shell_exec.phpt | 10 +- thirdparty/php/standard/proc_open.cc | 17 +- 18 files changed, 341 insertions(+), 219 deletions(-) diff --git a/core-tests/src/coroutine/system.cpp b/core-tests/src/coroutine/system.cpp index 0a39762d454..0552a909bfe 100644 --- a/core-tests/src/coroutine/system.cpp +++ b/core-tests/src/coroutine/system.cpp @@ -269,3 +269,12 @@ TEST(coroutine_system, timeout_is_zero) { ASSERT_TRUE(result); }); } + +TEST(coroutine_system, exec) { + test::coroutine::run([](void *arg) { + int status; + auto buffer = std::shared_ptr(swoole::make_string(1024)); + ASSERT_TRUE(System::exec("ls /", true, buffer, &status)); + ASSERT_TRUE(buffer->contains(SW_STRL("tmp"))); + }); +} diff --git a/core-tests/src/os/wait.cpp b/core-tests/src/os/wait.cpp index 431c30c5682..84e94689b8a 100644 --- a/core-tests/src/os/wait.cpp +++ b/core-tests/src/os/wait.cpp @@ -2,17 +2,34 @@ using namespace swoole; using namespace swoole::test; +using swoole::coroutine::System; -TEST(os_wait, waitpid_before_child_exit) { - test::coroutine::run([](void *arg) { - pid_t pid = fork(); - ASSERT_NE(pid, -1); +static pid_t fork_child() { + pid_t pid = fork(); + EXPECT_NE(pid, -1); - if (pid == 0) { - usleep(100000); - exit(0); - } + if (pid == 0) { + usleep(100000); + exit(0); + } + return pid; +} + +static pid_t fork_child2() { + pid_t pid = fork(); + EXPECT_NE(pid, -1); + if (pid == 0) { + exit(0); + } + + usleep(100000); + return pid; +} + +TEST(os_wait, waitpid_before_child_exit) { + test::coroutine::run([](void *arg) { + auto pid = fork_child(); int status = -1; pid_t pid2 = swoole_coroutine_waitpid(pid, &status, 0); ASSERT_EQ(status, 0); @@ -22,14 +39,7 @@ TEST(os_wait, waitpid_before_child_exit) { TEST(os_wait, waitpid_after_child_exit) { test::coroutine::run([](void *arg) { - pid_t pid = fork(); - ASSERT_NE(pid, -1); - - if (pid == 0) { - exit(0); - } - - usleep(100000); + pid_t pid = fork_child2(); int status = -1; pid_t pid2 = swoole_coroutine_waitpid(pid, &status, 0); ASSERT_EQ(status, 0); @@ -39,14 +49,7 @@ TEST(os_wait, waitpid_after_child_exit) { TEST(os_wait, wait_before_child_exit) { test::coroutine::run([](void *arg) { - pid_t pid = fork(); - ASSERT_NE(pid, -1); - - if (pid == 0) { - usleep(100000); - exit(0); - } - + pid_t pid = fork_child(); int status = -1; pid_t pid2 = -1; @@ -63,14 +66,7 @@ TEST(os_wait, wait_before_child_exit) { TEST(os_wait, wait_after_child_exit) { test::coroutine::run([](void *arg) { - pid_t pid = fork(); - ASSERT_NE(pid, -1); - - if (pid == 0) { - exit(0); - } - - usleep(100000); + pid_t pid = fork_child2(); int status = -1; pid_t pid2 = -1; @@ -84,3 +80,14 @@ TEST(os_wait, wait_after_child_exit) { ASSERT_EQ(WEXITSTATUS(status), 0); }); } + +TEST(os_wait, waitpid_safe) { + test::coroutine::run([](void *arg) { + pid_t pid = fork_child2(); + int status = -1; + + pid_t pid2 = System::waitpid_safe(pid, &status, 0); + ASSERT_EQ(pid2, pid); + ASSERT_EQ(WEXITSTATUS(status), 0); + }); +} diff --git a/ext-src/php_swoole.cc b/ext-src/php_swoole.cc index 80f5d3b3c23..8f8c8de8b09 100644 --- a/ext-src/php_swoole.cc +++ b/ext-src/php_swoole.cc @@ -1551,6 +1551,14 @@ static PHP_FUNCTION(swoole_implicit_fn) { abort(); } else if (SW_STRCASEEQ(fn, l_fn, "refcount")) { RETURN_LONG(zval_refcount_p(zargs)); + } else if (SW_STRCASEEQ(fn, l_fn, "func_handler")) { + auto fn = zval_get_string(zargs); + zend_function *zf = (zend_function *) zend_hash_find_ptr(EG(function_table), fn); + zend_string_release(fn); + if (zf == nullptr) { + RETURN_FALSE; + } + printf("zif_handler=%p\n", zf->internal_function.handler); } else { zend_throw_exception_ex(swoole_exception_ce, SW_ERROR_INVALID_PARAMS, "unknown fn '%s'", fn); } diff --git a/ext-src/php_swoole_cxx.h b/ext-src/php_swoole_cxx.h index 403057bd436..762a4dce2b9 100644 --- a/ext-src/php_swoole_cxx.h +++ b/ext-src/php_swoole_cxx.h @@ -638,40 +638,61 @@ class Callable { } }; -template +#define _CONCURRENCY_HASHMAP_LOCK_(code) \ + if (locked_) { \ + code; \ + } else { \ + lock_.lock(); \ + code; \ + lock_.unlock(); \ + } + +template class ConcurrencyHashMap { - private: + private: std::unordered_map map_; std::mutex lock_; + bool locked_; ValueT default_value_; - public: - ConcurrencyHashMap(ValueT _default_value): map_(), lock_() { + public: + ConcurrencyHashMap(ValueT _default_value) : map_(), lock_() { default_value_ = _default_value; + locked_ = false; } void set(const KeyT &key, const ValueT &value) { - std::unique_lock _lock(lock_); - map_[key] = value; + _CONCURRENCY_HASHMAP_LOCK_(map_[key] = value); } ValueT get(const KeyT &key) { - std::unique_lock _lock(lock_); - auto iter = map_.find(key); - if (iter == map_.end()) { - return default_value_; - } - return iter->second; + ValueT value; + auto fn = [&]() -> ValueT { + auto iter = map_.find(key); + if (iter == map_.end()) { + return default_value_; + } + return iter->second; + }; + _CONCURRENCY_HASHMAP_LOCK_(value = fn()); + return value; } void del(const KeyT &key) { - std::unique_lock _lock(lock_); - map_.erase(key); + _CONCURRENCY_HASHMAP_LOCK_(map_.erase(key)); } void clear() { + _CONCURRENCY_HASHMAP_LOCK_(map_.clear()); + } + + void each(const std::function &cb) { std::unique_lock _lock(lock_); - map_.clear(); + locked_ = true; + for (auto &iter : map_) { + cb(iter.first, iter.second); + } + locked_ = false; } }; diff --git a/ext-src/stubs/php_swoole_runtime.stub.php b/ext-src/stubs/php_swoole_runtime.stub.php index 00f13b65534..02aa74fb8cf 100644 --- a/ext-src/stubs/php_swoole_runtime.stub.php +++ b/ext-src/stubs/php_swoole_runtime.stub.php @@ -1,7 +1,7 @@ (swoole::make_string(1024, sw_zend_string_allocator())); + if (!System::exec(command, get_error_stream, buffer, &status)) { RETURN_FALSE; } - String *buffer = new String(1024); - Socket socket(fd, SW_SOCK_UNIX_STREAM); - while (1) { - ssize_t retval = socket.read(buffer->str + buffer->length, buffer->size - buffer->length); - if (retval > 0) { - buffer->length += retval; - if (buffer->length == buffer->size) { - if (!buffer->extend()) { - break; - } - } - } else { - break; - } - } - socket.close(); + auto str = zend::fetch_zend_string_by_val(buffer->str); + buffer->set_null_terminated(); + str->len = buffer->length; + buffer->release(); zval zdata; - if (buffer->length == 0) { - ZVAL_EMPTY_STRING(&zdata); - } else { - ZVAL_STRINGL(&zdata, buffer->str, buffer->length); - } - delete buffer; + ZVAL_STR(&zdata, str); - int status; - pid_t _pid = swoole_coroutine_waitpid(pid, &status, 0); - if (_pid > 0) { - array_init(return_value); - add_assoc_long(return_value, "code", WEXITSTATUS(status)); - add_assoc_long(return_value, "signal", WTERMSIG(status)); - add_assoc_zval(return_value, "output", &zdata); - } else { - zval_ptr_dtor(&zdata); - RETVAL_FALSE; - } + array_init(return_value); + add_assoc_long(return_value, "code", WEXITSTATUS(status)); + add_assoc_long(return_value, "signal", WTERMSIG(status)); + add_assoc_zval(return_value, "output", &zdata); } static void swoole_coroutine_system_wait(INTERNAL_FUNCTION_PARAMETERS, pid_t pid, double timeout) { @@ -310,6 +278,7 @@ static void swoole_coroutine_system_wait(INTERNAL_FUNCTION_PARAMETERS, pid_t pid } PHP_METHOD(swoole_coroutine_system, wait) { + SW_MUST_BE_MAIN_THREAD(); double timeout = -1; ZEND_PARSE_PARAMETERS_START(0, 1) @@ -321,6 +290,7 @@ PHP_METHOD(swoole_coroutine_system, wait) { } PHP_METHOD(swoole_coroutine_system, waitPid) { + SW_MUST_BE_MAIN_THREAD(); zend_long pid; double timeout = -1; diff --git a/ext-src/swoole_runtime.cc b/ext-src/swoole_runtime.cc index fe62b7e2673..bd5d43f6332 100644 --- a/ext-src/swoole_runtime.cc +++ b/ext-src/swoole_runtime.cc @@ -165,6 +165,39 @@ static zend_internal_arg_info *get_arginfo(const char *name, size_t l_name) { return zf->internal_function.arg_info; } +static zend_internal_arg_info *copy_arginfo(zend_function *zf, zend_internal_arg_info *_arg_info) { + uint32_t num_args = zf->internal_function.num_args + 1; + zend_internal_arg_info *arg_info = _arg_info - 1; + + auto new_arg_info = (zend_internal_arg_info *) pemalloc(sizeof(zend_internal_arg_info) * num_args, 1); + memcpy(new_arg_info, arg_info, sizeof(zend_internal_arg_info) * num_args); + + if (zf->internal_function.fn_flags & ZEND_ACC_VARIADIC) { + num_args++; + } + + for (uint32_t i = 0; i < num_args; i++) { + if (ZEND_TYPE_HAS_LIST(arg_info[i].type)) { + zend_type_list *old_list = ZEND_TYPE_LIST(arg_info[i].type); + zend_type_list *new_list = (zend_type_list *) pemalloc(ZEND_TYPE_LIST_SIZE(old_list->num_types), 1); + memcpy(new_list, old_list, ZEND_TYPE_LIST_SIZE(old_list->num_types)); + ZEND_TYPE_SET_PTR(new_arg_info[i].type, new_list); + + zend_type *list_type; + ZEND_TYPE_LIST_FOREACH(new_list, list_type) { + zend_string *name = zend_string_dup(ZEND_TYPE_NAME(*list_type), 1); + ZEND_TYPE_SET_PTR(*list_type, name); + } + ZEND_TYPE_LIST_FOREACH_END(); + } else if (ZEND_TYPE_HAS_NAME(arg_info[i].type)) { + zend_string *name = zend_string_dup(ZEND_TYPE_NAME(arg_info[i].type), 1); + ZEND_TYPE_SET_PTR(new_arg_info[i].type, name); + } + } + + return new_arg_info + 1; +} + #define SW_HOOK_FUNC(f) hook_func(ZEND_STRL(#f), PHP_FN(swoole_##f)) #define SW_UNHOOK_FUNC(f) unhook_func(ZEND_STRL(#f)) #define SW_HOOK_WITH_NATIVE_FUNC(f) \ @@ -175,7 +208,7 @@ static zend_internal_arg_info *get_arginfo(const char *name, size_t l_name) { ZEND_RAW_FENTRY("swoole_hook_" #name, PHP_FN(swoole_user_func_handler), arg_info, 0) static bool runtime_hook_init = false; -static int runtime_hook_flags = 0; +static SW_THREAD_LOCAL int runtime_hook_flags = 0; static SW_THREAD_LOCAL zend_array *tmp_function_table = nullptr; static SW_THREAD_LOCAL std::unordered_map child_class_entries; static zend::ConcurrencyHashMap ori_func_handlers(nullptr); @@ -1194,29 +1227,17 @@ void PHPCoroutine::enable_unsafe_function() { } } -bool PHPCoroutine::enable_hook(uint32_t flags) { - SW_MUST_BE_MAIN_THREAD_EX(return false); - if (swoole_isset_hook((enum swGlobalHookType) PHP_SWOOLE_HOOK_BEFORE_ENABLE_HOOK)) { - swoole_call_hook((enum swGlobalHookType) PHP_SWOOLE_HOOK_BEFORE_ENABLE_HOOK, &flags); - } - +static void hook_stream_factory(int flags) { + HashTable *xport_hash = php_stream_xport_get_hash(); if (!runtime_hook_init) { - HashTable *xport_hash = php_stream_xport_get_hash(); - // php_stream ori_factory.tcp = (php_stream_transport_factory) zend_hash_str_find_ptr(xport_hash, ZEND_STRL("tcp")); ori_factory.udp = (php_stream_transport_factory) zend_hash_str_find_ptr(xport_hash, ZEND_STRL("udp")); ori_factory._unix = (php_stream_transport_factory) zend_hash_str_find_ptr(xport_hash, ZEND_STRL("unix")); ori_factory.udg = (php_stream_transport_factory) zend_hash_str_find_ptr(xport_hash, ZEND_STRL("udg")); ori_factory.ssl = (php_stream_transport_factory) zend_hash_str_find_ptr(xport_hash, ZEND_STRL("ssl")); ori_factory.tls = (php_stream_transport_factory) zend_hash_str_find_ptr(xport_hash, ZEND_STRL("tls")); - - // file - memcpy((void *) &ori_php_plain_files_wrapper, &php_plain_files_wrapper, sizeof(php_plain_files_wrapper)); - memcpy((void *) &ori_php_stream_stdio_ops, &php_stream_stdio_ops, sizeof(php_stream_stdio_ops)); - - runtime_hook_init = true; } - // php_stream + if (flags & PHPCoroutine::HOOK_TCP) { if (!(runtime_hook_flags & PHPCoroutine::HOOK_TCP)) { if (php_stream_xport_register("tcp", socket_create) != SUCCESS) { @@ -1291,6 +1312,36 @@ bool PHPCoroutine::enable_hook(uint32_t flags) { } } } +} + +static void hook_stream_ops(int flags) { + if (!runtime_hook_init) { + memcpy((void *) &ori_php_plain_files_wrapper, &php_plain_files_wrapper, sizeof(php_plain_files_wrapper)); + memcpy((void *) &ori_php_stream_stdio_ops, &php_stream_stdio_ops, sizeof(php_stream_stdio_ops)); + } + // file + if (flags & PHPCoroutine::HOOK_FILE) { + if (!(runtime_hook_flags & PHPCoroutine::HOOK_FILE)) { + memcpy((void *) &php_plain_files_wrapper, &sw_php_plain_files_wrapper, sizeof(php_plain_files_wrapper)); + } + } else { + if (runtime_hook_flags & PHPCoroutine::HOOK_FILE) { + memcpy((void *) &php_plain_files_wrapper, &ori_php_plain_files_wrapper, sizeof(php_plain_files_wrapper)); + } + } + // stdio + if (flags & PHPCoroutine::HOOK_STDIO) { + if (!(runtime_hook_flags & PHPCoroutine::HOOK_STDIO)) { + memcpy((void *) &php_stream_stdio_ops, &sw_php_stream_stdio_ops, sizeof(php_stream_stdio_ops)); + } + } else { + if (runtime_hook_flags & PHPCoroutine::HOOK_STDIO) { + memcpy((void *) &php_stream_stdio_ops, &ori_php_stream_stdio_ops, sizeof(php_stream_stdio_ops)); + } + } +} + +static void hook_pdo_driver(int flags) { #ifdef SW_USE_PGSQL if (flags & PHPCoroutine::HOOK_PDO_PGSQL) { if (!(runtime_hook_flags & PHPCoroutine::HOOK_PDO_PGSQL)) { @@ -1335,6 +1386,10 @@ bool PHPCoroutine::enable_hook(uint32_t flags) { } } #endif +} + +static void hook_all_func(int flags) { + // stream func if (flags & PHPCoroutine::HOOK_STREAM_FUNCTION) { if (!(runtime_hook_flags & PHPCoroutine::HOOK_STREAM_FUNCTION)) { SW_HOOK_FUNC(stream_select); @@ -1346,26 +1401,6 @@ bool PHPCoroutine::enable_hook(uint32_t flags) { SW_UNHOOK_FUNC(stream_socket_pair); } } - // file - if (flags & PHPCoroutine::HOOK_FILE) { - if (!(runtime_hook_flags & PHPCoroutine::HOOK_FILE)) { - memcpy((void *) &php_plain_files_wrapper, &sw_php_plain_files_wrapper, sizeof(php_plain_files_wrapper)); - } - } else { - if (runtime_hook_flags & PHPCoroutine::HOOK_FILE) { - memcpy((void *) &php_plain_files_wrapper, &ori_php_plain_files_wrapper, sizeof(php_plain_files_wrapper)); - } - } - // stdio - if (flags & PHPCoroutine::HOOK_STDIO) { - if (!(runtime_hook_flags & PHPCoroutine::HOOK_STDIO)) { - memcpy((void *) &php_stream_stdio_ops, &sw_php_stream_stdio_ops, sizeof(php_stream_stdio_ops)); - } - } else { - if (runtime_hook_flags & PHPCoroutine::HOOK_STDIO) { - memcpy((void *) &php_stream_stdio_ops, &ori_php_stream_stdio_ops, sizeof(php_stream_stdio_ops)); - } - } // sleep if (flags & PHPCoroutine::HOOK_SLEEP) { if (!(runtime_hook_flags & PHPCoroutine::HOOK_SLEEP)) { @@ -1402,8 +1437,8 @@ bool PHPCoroutine::enable_hook(uint32_t flags) { if (flags & PHPCoroutine::HOOK_BLOCKING_FUNCTION) { if (!(runtime_hook_flags & PHPCoroutine::HOOK_BLOCKING_FUNCTION)) { hook_func(ZEND_STRL("gethostbyname"), PHP_FN(swoole_coroutine_gethostbyname)); - hook_func(ZEND_STRL("exec")); - hook_func(ZEND_STRL("shell_exec")); + SW_HOOK_WITH_PHP_FUNC(exec); + SW_HOOK_WITH_PHP_FUNC(shell_exec); } } else { if (runtime_hook_flags & PHPCoroutine::HOOK_BLOCKING_FUNCTION) { @@ -1412,6 +1447,7 @@ bool PHPCoroutine::enable_hook(uint32_t flags) { SW_UNHOOK_FUNC(shell_exec); } } + // ext-sockets if (flags & PHPCoroutine::HOOK_SOCKETS) { if (!(runtime_hook_flags & PHPCoroutine::HOOK_SOCKETS)) { SW_HOOK_WITH_PHP_FUNC(socket_create); @@ -1477,6 +1513,7 @@ bool PHPCoroutine::enable_hook(uint32_t flags) { } #ifdef SW_USE_CURL + // curl native if (flags & PHPCoroutine::HOOK_NATIVE_CURL) { if (flags & PHPCoroutine::HOOK_CURL) { php_swoole_fatal_error(E_WARNING, "cannot enable both hooks HOOK_NATIVE_CURL and HOOK_CURL at same time"); @@ -1537,7 +1574,7 @@ bool PHPCoroutine::enable_hook(uint32_t flags) { } } #endif - + // curl if (flags & PHPCoroutine::HOOK_CURL) { if (!(runtime_hook_flags & PHPCoroutine::HOOK_CURL)) { SW_HOOK_WITH_PHP_FUNC(curl_init); @@ -1569,12 +1606,32 @@ bool PHPCoroutine::enable_hook(uint32_t flags) { detach_parent_class("Swoole\\Curl\\Handler"); } } +} + +bool PHPCoroutine::enable_hook(uint32_t flags) { + if (swoole_isset_hook((enum swGlobalHookType) PHP_SWOOLE_HOOK_BEFORE_ENABLE_HOOK)) { + swoole_call_hook((enum swGlobalHookType) PHP_SWOOLE_HOOK_BEFORE_ENABLE_HOOK, &flags); + } + + /** + * These resources are global variables that can only be modified once within the main thread, + * and such modifications are not thread-safe. + */ + if (sw_is_main_thread()) { + hook_stream_factory(flags); + hook_pdo_driver(flags); + hook_stream_ops(flags); + } + + hook_all_func(flags); if (swoole_isset_hook((enum swGlobalHookType) PHP_SWOOLE_HOOK_AFTER_ENABLE_HOOK)) { swoole_call_hook((enum swGlobalHookType) PHP_SWOOLE_HOOK_AFTER_ENABLE_HOOK, &flags); } + runtime_hook_init = true; runtime_hook_flags = flags; + return true; } @@ -1587,34 +1644,13 @@ static PHP_METHOD(swoole_runtime, enableCoroutine) { php_swoole_fatal_error(E_ERROR, "must be used in PHP CLI mode"); RETURN_FALSE; } - zval *zflags = nullptr; zend_long flags = PHPCoroutine::HOOK_ALL; - ZEND_PARSE_PARAMETERS_START(0, 2) + ZEND_PARSE_PARAMETERS_START(0, 1) Z_PARAM_OPTIONAL - Z_PARAM_ZVAL(zflags) // or zenable Z_PARAM_LONG(flags) ZEND_PARSE_PARAMETERS_END_EX(RETURN_FALSE); - if (zflags) { - if (Z_TYPE_P(zflags) == IS_LONG) { - flags = SW_MAX(0, Z_LVAL_P(zflags)); - } else if (ZVAL_IS_BOOL(zflags)) { - if (!Z_BVAL_P(zflags)) { - flags = 0; - } - } else { - const char *space, *class_name = get_active_class_name(&space); - zend_type_error("%s%s%s() expects parameter %d to be %s, %s given", - class_name, - space, - get_active_function_name(), - 1, - "bool or long", - zend_zval_type_name(zflags)); - } - } - #ifdef SW_THREAD if (runtime_hook_init && flags == 0) { swoole_set_last_error(SW_ERROR_OPERATION_NOT_SUPPORT); @@ -1996,25 +2032,17 @@ static void hook_func(const char *name, size_t l_name, zif_handler handler, zend auto fn_name = std::string(fn_str->val, fn_str->len); + rf->ori_handler = zf->internal_function.handler; + rf->ori_arg_info = zf->internal_function.arg_info; + if (sw_is_main_thread()) { - rf->ori_handler = zf->internal_function.handler; - rf->ori_arg_info = zf->internal_function.arg_info; - /** - * The internal functions differ from user-defined functions in that they are shared among multiple threads. - * When the function handle is replaced in the main thread, - * the child threads will call the hook handle instead of the original handle. - * User-defined functions need to be reconstructed in the child threads. - */ ori_func_handlers.set(fn_name, rf->ori_handler); ori_func_arg_infos.set(fn_name, rf->ori_arg_info); + } - zf->internal_function.handler = handler; - if (arg_info) { - zf->internal_function.arg_info = arg_info; - } - } else { - rf->ori_handler = ori_func_handlers.get(fn_name); - rf->ori_arg_info = ori_func_arg_infos.get(fn_name); + zf->internal_function.handler = handler; + if (arg_info) { + zf->internal_function.arg_info = copy_arginfo(zf, arg_info); } if (use_php_func) { @@ -2080,13 +2108,21 @@ php_stream_ops *php_swoole_get_ori_php_stream_stdio_ops() { zif_handler php_swoole_get_original_handler(const char *name, size_t len) { if (sw_is_main_thread()) { real_func *rf = (real_func *) zend_hash_str_find_ptr(tmp_function_table, name, len); - if (!rf) { - return nullptr; + if (rf) { + return rf->ori_handler; } - return rf->ori_handler; } else { - return ori_func_handlers.get(std::string(name, len)); + zif_handler handler = ori_func_handlers.get(std::string(name, len)); + if (handler) { + return handler; + } + zend_function *zf = (zend_function *) zend_hash_str_find_ptr(EG(function_table), name, len); + if (zf && zf->type == ZEND_INTERNAL_FUNCTION && zf->internal_function.handler) { + return zf->internal_function.handler; + } } + + return nullptr; } static PHP_FUNCTION(swoole_stream_socket_pair) { @@ -2131,17 +2167,8 @@ static PHP_FUNCTION(swoole_user_func_handler) { real_func *rf = (real_func *) zend_hash_find_ptr(tmp_function_table, fn_str); if (!rf) { -#ifdef SW_THREAD - /** - * The Callable object with ZTS needs to be reconstructed for each thread, - * ensuring that each thread is isolated from the others. - */ - hook_func(fn_str->val, fn_str->len); - rf = (real_func *) zend_hash_find_ptr(tmp_function_table, fn_str); -#else zend_throw_exception_ex(swoole_exception_ce, SW_ERROR_UNDEFINED_BEHAVIOR, "%s func not exists", fn_str->val); return; -#endif } zend_fcall_info fci; diff --git a/include/swoole_coroutine.h b/include/swoole_coroutine.h index 424053ad58d..ff19aff2ad2 100644 --- a/include/swoole_coroutine.h +++ b/include/swoole_coroutine.h @@ -317,6 +317,7 @@ bool async(async::Handler handler, AsyncEvent &event, double timeout = -1); */ bool async(const std::function &fn); bool run(const CoroutineFunc &fn, void *arg = nullptr); +bool wait_for(const std::function &fn); } // namespace coroutine //------------------------------------------------------------------------------- } // namespace swoole diff --git a/include/swoole_coroutine_system.h b/include/swoole_coroutine_system.h index d552648e5a8..e55d9913ec3 100644 --- a/include/swoole_coroutine_system.h +++ b/include/swoole_coroutine_system.h @@ -66,11 +66,17 @@ class System { /* wait */ static pid_t wait(int *__stat_loc, double timeout = -1); static pid_t waitpid(pid_t __pid, int *__stat_loc, int __options, double timeout = -1); + /** + * waitpid_safe() does not deps on the signal + * and can be safely used in a multi-threaded environment. + */ + static pid_t waitpid_safe(pid_t __pid, int *__stat_loc, int __options); /* signal */ static int wait_signal(int signal, double timeout = -1); static int wait_signal(const std::vector &signals, double timeout = -1); /* event */ static int wait_event(int fd, int events, double timeout); + static bool exec(const char *command, bool get_error_stream, std::shared_ptr buffer, int *status); }; std::string gethostbyname_impl_with_async(const std::string &hostname, int domain, double timeout = -1); //------------------------------------------------------------------------------- diff --git a/src/coroutine/system.cc b/src/coroutine/system.cc index 39c70f82f6e..cc6d5856eb8 100644 --- a/src/coroutine/system.cc +++ b/src/coroutine/system.cc @@ -244,8 +244,8 @@ int System::wait_signal(int signal, double timeout) { */ int System::wait_signal(const std::vector &signals, double timeout) { SignalListener listener = { - Coroutine::get_current_safe(), - -1, + Coroutine::get_current_safe(), + -1, }; if (SwooleTG.signal_listener_num > 0) { @@ -589,6 +589,35 @@ int System::wait_event(int fd, int events, double timeout) { return revents; } +bool System::exec(const char *command, bool get_error_stream, std::shared_ptr buffer, int *status) { + Coroutine::get_current_safe(); + + pid_t pid; + int fd = swoole_shell_exec(command, &pid, get_error_stream); + if (fd < 0) { + swoole_sys_warning("Unable to execute '%s'", command); + return false; + } + + Socket socket(fd, SW_SOCK_UNIX_STREAM); + while (1) { + ssize_t retval = socket.read(buffer->str + buffer->length, buffer->size - buffer->length); + if (retval > 0) { + buffer->length += retval; + if (buffer->length == buffer->size) { + if (!buffer->extend()) { + break; + } + } + } else { + break; + } + } + socket.close(); + + return System::waitpid_safe(pid, status, 0) == pid; +} + void System::init_reactor(Reactor *reactor) { reactor->set_handler(SW_FD_CO_POLL | SW_EVENT_READ, socket_poll_read_callback); reactor->set_handler(SW_FD_CO_POLL | SW_EVENT_WRITE, socket_poll_write_callback); @@ -698,5 +727,19 @@ std::shared_ptr async_lock(void *resource) { return std::make_shared(resource); } +bool wait_for(const std::function &fn) { + double second = 0.001; + while (true) { + if (fn()) { + break; + } + if (System::sleep(second) != SW_OK) { + return false; + } + second *= 2; + } + return true; +} + } // namespace coroutine } // namespace swoole diff --git a/src/os/wait.cc b/src/os/wait.cc index bd2df21d422..ca02eee1388 100644 --- a/src/os/wait.cc +++ b/src/os/wait.cc @@ -32,9 +32,13 @@ struct WaitTask { int status; }; +/** + * Wait, waitpid, and signal cannot be used in a multi-threaded environment; + * they are only applicable to the main thread. There is no need to treat them as thread-local variables. + */ static std::list wait_list; -static std::unordered_map waitpid_map; -static std::unordered_map child_processes; +static std::unordered_map waitpid_map; +static std::unordered_map child_processes; bool signal_ready = false; @@ -88,6 +92,20 @@ pid_t System::wait(int *__stat_loc, double timeout) { return System::waitpid(-1, __stat_loc, 0, timeout); } +pid_t System::waitpid_safe(pid_t __pid, int *__stat_loc, int __options) { + if (sw_unlikely(SwooleTG.reactor == nullptr || !Coroutine::get_current() || (__options & WNOHANG))) { + return ::waitpid(__pid, __stat_loc, __options); + } + + pid_t retval; + auto success = wait_for([__pid, &retval, __stat_loc]() -> bool { + retval = ::waitpid(__pid, __stat_loc, WNOHANG); + return retval != 0; + }); + + return success ? retval : -1; +} + /** * @error: errno & swoole_get_last_error() */ @@ -118,7 +136,7 @@ pid_t System::waitpid(pid_t __pid, int *__stat_loc, int __options, double timeou WaitTask task; signal_init(); task.pid = ::waitpid(__pid, __stat_loc, __options | WNOHANG); - if (task.pid > 0) { + if (task.pid != 0) { return task.pid; } diff --git a/tests/swoole_coroutine/cancel/wait.phpt b/tests/swoole_coroutine/cancel/wait.phpt index a847f978e4c..75b24b78723 100644 --- a/tests/swoole_coroutine/cancel/wait.phpt +++ b/tests/swoole_coroutine/cancel/wait.phpt @@ -6,19 +6,27 @@ swoole_coroutine/cancel: wait/waitpid start(); + +run(function () use ($proc) { $cid = Coroutine::getCid(); go(function () use ($cid) { System::sleep(0.002); Assert::true(Coroutine::cancel($cid)); }); + $retval = System::wait(); echo "Done\n"; + Process::kill($proc->pid, SIGKILL); Assert::eq($retval, false); Assert::eq(swoole_last_error(), SWOOLE_ERROR_CO_CANCELED); }); diff --git a/tests/swoole_runtime/base.phpt b/tests/swoole_runtime/base.phpt index ef2e31fef0a..9f907e3edda 100644 --- a/tests/swoole_runtime/base.phpt +++ b/tests/swoole_runtime/base.phpt @@ -7,7 +7,7 @@ swoole_runtime: base require __DIR__ . '/../include/bootstrap.php'; $server = SwooleTest\CoServer::createTcpGreeting(); $server->run(); -Swoole\Runtime::enableCoroutine(true, SWOOLE_HOOK_ALL ^ SWOOLE_HOOK_SLEEP); +Swoole\Runtime::enableCoroutine(SWOOLE_HOOK_ALL ^ SWOOLE_HOOK_SLEEP); go(function () { usleep(1000); echo '1' . PHP_EOL; @@ -42,7 +42,7 @@ go(function () use ($server) { $server->shutdown(); }); echo '5' . PHP_EOL; -Swoole\Runtime::enableCoroutine(true); // all +Swoole\Runtime::enableCoroutine(SWOOLE_HOOK_ALL); // all go(function () { usleep(5 * 1000); echo 'sleep1' . PHP_EOL; @@ -57,7 +57,7 @@ go(function () use ($server) { }); echo '7' . PHP_EOL; Swoole\Event::wait(); -Swoole\Runtime::enableCoroutine(false); // disable all +Swoole\Runtime::enableCoroutine(0); // disable all ?> --EXPECT-- 1 diff --git a/tests/swoole_runtime/file_hook/bug_4327.phpt b/tests/swoole_runtime/file_hook/bug_4327.phpt index ca200abb0d0..1eaa823f2c6 100644 --- a/tests/swoole_runtime/file_hook/bug_4327.phpt +++ b/tests/swoole_runtime/file_hook/bug_4327.phpt @@ -15,44 +15,46 @@ require __DIR__.'/../../include/bootstrap.php'; Swoole\Runtime::enableCoroutine($flags = SWOOLE_HOOK_ALL); +const __ROOT_DIR = 'tmp/'; + function createDirectories($protocol = "") { $barrier = Barrier::make(); - $first = "$protocol/".rand(0, 1000); - $second = "/".rand(0, 1000); - $third = "/".rand(0, 1000)."/"; + $first = "$protocol/" . __ROOT_DIR . rand(0, 1000); + $second = "/" . rand(0, 1000); + $third = "/" . rand(0, 1000) . "/"; for ($i = 0; $i < 5; $i++) { Coroutine::create(static function () use ($i, $first, $second, $third, $barrier) { - if (!mkdir($directory = $first.$second.$third.$i, 0755, true) && !is_dir($directory)) { + if (!mkdir($directory = $first . $second . $third . $i, 0755, true) && !is_dir($directory)) { throw new Exception("create directory failed"); } rmdir($directory); }); } - echo "SUCCESS".PHP_EOL; + echo "SUCCESS" . PHP_EOL; Barrier::wait($barrier); - rmdir($first.$second.$third); - rmdir($first.$second); + rmdir($first . $second . $third); + rmdir($first . $second); rmdir($first); } - run(function () { createDirectories(); createDirectories("file://"); }); if (defined('SWOOLE_THREAD')) { - echo "SUCCESS".PHP_EOL; - echo "SUCCESS".PHP_EOL; + echo "SUCCESS" . PHP_EOL; + echo "SUCCESS" . PHP_EOL; } else { - Swoole\Runtime::enableCoroutine(false); - createDirectories(); - createDirectories("file://"); + run(function () { + Swoole\Runtime::enableCoroutine(false); + createDirectories(); + createDirectories("file://"); + }); } - ?> --EXPECT-- SUCCESS diff --git a/tests/swoole_thread/pipe.phpt b/tests/swoole_thread/pipe.phpt index 7b885e99c9d..10875bffe30 100644 --- a/tests/swoole_thread/pipe.phpt +++ b/tests/swoole_thread/pipe.phpt @@ -25,8 +25,6 @@ if (empty($args)) { } else { $socket = $args[0]; $rdata = $args[1]; - // Child threads are not allowed to modify hook flags - Assert::false(Swoole\Runtime::enableCoroutine(SWOOLE_HOOK_ALL)); Co\run(function () use ($socket, $rdata, $argv) { usleep(100); shell_exec('sleep 0.01'); diff --git a/tests/swoole_thread/shell_exec.phpt b/tests/swoole_thread/shell_exec.phpt index 73a5ae308c8..f3a7e44920c 100644 --- a/tests/swoole_thread/shell_exec.phpt +++ b/tests/swoole_thread/shell_exec.phpt @@ -14,12 +14,10 @@ use Swoole\Thread\Lock; use Swoole\Runtime; use SwooleTest\ThreadManager; -const CODE = 234; - $tm = new ThreadManager(); $tm->parentFunc = function () { - Runtime::enableCoroutine(SWOOLE_HOOK_ALL); + Assert::true(Runtime::enableCoroutine(SWOOLE_HOOK_ALL)); $lock = new Lock; $lock->lock(); $thread = new Thread(__FILE__, $lock); @@ -32,9 +30,11 @@ $tm->parentFunc = function () { $tm->childFunc = function ($lock) { $lock->lock(); usleep(100_000); -// shell_exec('ls /tmp'); - Co\run(function (){ + Co\run(function () { + Assert::true(Runtime::enableCoroutine(SWOOLE_HOOK_ALL)); shell_exec('ls /tmp'); + sleep(1); + gethostbyname('www.baidu.com'); }); exit(0); }; diff --git a/thirdparty/php/standard/proc_open.cc b/thirdparty/php/standard/proc_open.cc index 4363be6b0e2..a494f795401 100644 --- a/thirdparty/php/standard/proc_open.cc +++ b/thirdparty/php/standard/proc_open.cc @@ -15,12 +15,12 @@ */ #include "thirdparty/php/standard/proc_open.h" -#include "swoole_coroutine_c_api.h" using namespace std; using swoole::Coroutine; using swoole::PHPCoroutine; using swoole::coroutine::Socket; +using swoole::coroutine::System; #ifdef HAVE_SYS_WAIT_H #include @@ -53,6 +53,14 @@ extern int openpty(int *, int *, char *, struct termios *, struct winsize *); static int le_proc_open; static const char *le_proc_name = "process/coroutine"; +static pid_t _co_waitpid(pid_t __pid, int *__stat_loc, int __options) { +#ifdef SW_THREAD + return System::waitpid_safe(__pid, __stat_loc, __options); +#else + return System::waitpid(__pid, __stat_loc, __options); +#endif +} + /* {{{ _php_array_to_envp * Process the `environment` argument to `proc_open` * Convert into data structures which can be passed to underlying OS APIs like `exec` on POSIX or @@ -170,9 +178,7 @@ static void proc_co_rsrc_dtor(zend_resource *rsrc) { } if (proc->running) { - if (::waitpid(proc->child, &wstatus, WNOHANG) == 0) { - swoole_coroutine_waitpid(proc->child, &wstatus, 0); - } + _co_waitpid(proc->child, &wstatus, 0); } if (proc->wstatus) { *proc->wstatus = wstatus; @@ -257,7 +263,7 @@ PHP_FUNCTION(swoole_proc_get_status) { add_assoc_long(return_value, "pid", (zend_long) proc->child); errno = 0; - wait_pid = swoole_coroutine_waitpid(proc->child, &wstatus, WNOHANG | WUNTRACED); + wait_pid = _co_waitpid(proc->child, &wstatus, WNOHANG | WUNTRACED); if (wait_pid == proc->child) { if (WIFEXITED(wstatus)) { @@ -1272,4 +1278,3 @@ PHP_FUNCTION(swoole_proc_open) { } } /* }}} */ -