From 050293e7ebaae86e9b1df295b2c1f03bb63905b0 Mon Sep 17 00:00:00 2001 From: matyhtf Date: Tue, 5 Nov 2024 14:16:40 +0800 Subject: [PATCH] Optimize ssl context code, fix tests, fix memory leak --- .../php_swoole_http_server_coro.stub.php | 2 +- .../php_swoole_http_server_coro_arginfo.h | 4 +-- ext-src/swoole_http_server_coro.cc | 26 +++++++++---------- ext-src/swoole_runtime.cc | 9 ++++--- include/swoole_coroutine_socket.h | 4 +-- src/core/base.cc | 6 +++++ src/coroutine/socket.cc | 18 +++++-------- tests/swoole_server_coro/ssl.phpt | 3 +-- 8 files changed, 36 insertions(+), 36 deletions(-) diff --git a/ext-src/stubs/php_swoole_http_server_coro.stub.php b/ext-src/stubs/php_swoole_http_server_coro.stub.php index 966604fd39f..e96a06f2643 100644 --- a/ext-src/stubs/php_swoole_http_server_coro.stub.php +++ b/ext-src/stubs/php_swoole_http_server_coro.stub.php @@ -4,7 +4,7 @@ final class Server { public function __construct(string $host, int $port = 0, bool $ssl = false, bool $reuse_port = false) {} public function __destruct() {} public function set(array $settings): bool {} - public function handle(string $pattern, callable $callback): void {} + public function handle(string $pattern, callable $callback): bool {} public function start(): bool {} public function shutdown(): void {} private function onAccept(\Swoole\Coroutine\Socket $conn): void {} diff --git a/ext-src/stubs/php_swoole_http_server_coro_arginfo.h b/ext-src/stubs/php_swoole_http_server_coro_arginfo.h index 2eb6815670e..7a8e9727f5e 100644 --- a/ext-src/stubs/php_swoole_http_server_coro_arginfo.h +++ b/ext-src/stubs/php_swoole_http_server_coro_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 2f5ecf154780c21ccc66ba5e2fd318eb117191b0 */ + * Stub hash: 6f65975475013861bdaec52a0fb3fe3b1dc75657 */ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Swoole_Coroutine_Http_Server___construct, 0, 0, 1) ZEND_ARG_TYPE_INFO(0, host, IS_STRING, 0) @@ -15,7 +15,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_Swoole_Coroutine_Http_Serv ZEND_ARG_TYPE_INFO(0, settings, IS_ARRAY, 0) ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_Swoole_Coroutine_Http_Server_handle, 0, 2, IS_VOID, 0) +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_Swoole_Coroutine_Http_Server_handle, 0, 2, _IS_BOOL, 0) ZEND_ARG_TYPE_INFO(0, pattern, IS_STRING, 0) ZEND_ARG_TYPE_INFO(0, callback, IS_CALLABLE, 0) ZEND_END_ARG_INFO() diff --git a/ext-src/swoole_http_server_coro.cc b/ext-src/swoole_http_server_coro.cc index b4b4b71edc5..2f3a18fe546 100644 --- a/ext-src/swoole_http_server_coro.cc +++ b/ext-src/swoole_http_server_coro.cc @@ -105,11 +105,19 @@ class HttpServer { delete socket; } - void set_handler(std::string pattern, zend::Callable *cb) { + bool set_handler(std::string pattern, zval *zfn) { + auto cb = sw_callable_create(zfn); + if (!cb) { + return false; + } + if (handlers.find(pattern) != handlers.end()) { + sw_callable_free(handlers[pattern]); + } handlers[pattern] = cb; if (pattern == "/") { default_handler = cb; } + return true; } zend::Callable *get_handler(HttpContext *ctx) { @@ -388,13 +396,8 @@ static PHP_METHOD(swoole_http_server_coro, handle) { Z_PARAM_ZVAL(zfn) ZEND_PARSE_PARAMETERS_END(); - auto cb = sw_callable_create(zfn); - if (!cb) { - RETURN_FALSE; - } - std::string key(pattern, pattern_len); - hs->set_handler(key, cb); + RETURN_BOOL(hs->set_handler(key, zfn)); } static PHP_METHOD(swoole_http_server_coro, set) { @@ -421,10 +424,9 @@ static PHP_METHOD(swoole_http_server_coro, start) { /* get callback fci cache */ char *func_name = nullptr; zend_fcall_info_cache fci_cache; - zval zcallback; - ZVAL_STRING(&zcallback, "onAccept"); + zend::Variable zcallback("onAccept"); if (!sw_zend_is_callable_at_frame( - &zcallback, ZEND_THIS, execute_data, 0, &func_name, nullptr, &fci_cache, nullptr)) { + zcallback.ptr(), ZEND_THIS, execute_data, 0, &func_name, nullptr, &fci_cache, nullptr)) { php_swoole_fatal_error(E_CORE_ERROR, "function '%s' is not callable", func_name); return; } @@ -510,7 +512,7 @@ static PHP_METHOD(swoole_http_server_coro, start) { if (conn) { zval zsocket; php_swoole_init_socket_object(&zsocket, conn); - long cid = PHPCoroutine::create(&fci_cache, 1, &zsocket, &zcallback); + long cid = PHPCoroutine::create(&fci_cache, 1, &zsocket, zcallback.ptr()); zval_dtor(&zsocket); if (cid < 0) { goto _wait_1s; @@ -535,8 +537,6 @@ static PHP_METHOD(swoole_http_server_coro, start) { } } - zval_dtor(&zcallback); - RETURN_TRUE; } diff --git a/ext-src/swoole_runtime.cc b/ext-src/swoole_runtime.cc index a8140b2beda..fe62b7e2673 100644 --- a/ext-src/swoole_runtime.cc +++ b/ext-src/swoole_runtime.cc @@ -786,10 +786,13 @@ static int socket_enable_crypto(php_stream *stream, Socket *sock, php_stream_xpo return -1; } - zval *val = php_stream_context_get_option(context, "ssl", "capture_peer_cert"); - if (val && zend_is_true(val) && !php_openssl_capture_peer_certs(stream, sock)) { - return -1; + if (context && sock->ssl_is_available()) { + zval *val = php_stream_context_get_option(context, "ssl", "capture_peer_cert"); + if (val && zend_is_true(val) && !php_openssl_capture_peer_certs(stream, sock)) { + return -1; + } } + return 1; } #endif diff --git a/include/swoole_coroutine_socket.h b/include/swoole_coroutine_socket.h index 1c1ec07f5e9..a26a57a153b 100644 --- a/include/swoole_coroutine_socket.h +++ b/include/swoole_coroutine_socket.h @@ -129,8 +129,7 @@ class Socket { * Operation sequence: * 1. enable_ssl_encrypt() * 2. Set SSL parameters, such as certificate file, key file - * 3. ssl_check_context() - * 4. ssl_accept()/ssl_connect()/ssl_handshake() + * 3. ssl_handshake(), to be executed after connect or accept */ bool enable_ssl_encrypt() { if (ssl_context.get()) { @@ -442,7 +441,6 @@ class Socket { std::string ssl_host_name; bool ssl_context_create(); bool ssl_create(SSLContext *ssl_context); - bool ssl_listen(); #endif bool connected = false; diff --git a/src/core/base.cc b/src/core/base.cc index 6d271ecb32b..d5ab76f905c 100644 --- a/src/core/base.cc +++ b/src/core/base.cc @@ -255,6 +255,12 @@ void swoole_clean(void) { delete SwooleTG.buffer_stack; SwooleTG.buffer_stack = nullptr; } + SW_LOOP_N(SW_MAX_HOOK_TYPE) { + if (SwooleG.hooks[i]) { + auto hooks = static_cast *>(SwooleG.hooks[i]); + delete hooks; + } + } swoole_signal_clear(); SwooleG = {}; } diff --git a/src/coroutine/socket.cc b/src/coroutine/socket.cc index 837068844fd..464780b3b21 100644 --- a/src/coroutine/socket.cc +++ b/src/coroutine/socket.cc @@ -1137,10 +1137,7 @@ bool Socket::listen(int backlog) { return false; } #ifdef SW_USE_OPENSSL - if (ssl_is_enable() && !ssl_listen()) { - set_err(SW_ERROR_SSL_CREATE_CONTEXT_FAILED); - return false; - } + ssl_is_server = true; #endif return true; } @@ -1149,6 +1146,11 @@ Socket *Socket::accept(double timeout) { if (sw_unlikely(!is_available(SW_EVENT_READ))) { return nullptr; } +#ifdef SW_USE_OPENSSL + if (ssl_is_enable() && sw_unlikely(ssl_context->context == nullptr) && !ssl_context_create()) { + return nullptr; + } +#endif network::Socket *conn = socket->accept(); if (conn == nullptr && errno == EAGAIN) { TimerController timer(&read_timer, timeout == 0 ? read_timeout : timeout, this, timer_callback); @@ -1213,14 +1215,6 @@ bool Socket::ssl_create(SSLContext *ssl_context) { return true; } -bool Socket::ssl_listen() { - ssl_is_server = true; - if (ssl_context->context == nullptr && !ssl_context_create()) { - return false; - } - return true; -} - bool Socket::ssl_handshake() { if (ssl_handshaked) { return false; diff --git a/tests/swoole_server_coro/ssl.phpt b/tests/swoole_server_coro/ssl.phpt index ea747b9a5c8..497fed99155 100644 --- a/tests/swoole_server_coro/ssl.phpt +++ b/tests/swoole_server_coro/ssl.phpt @@ -13,8 +13,7 @@ $pm = new ProcessManager; $pm->parentFunc = function ($pid) use ($pm) { $client = new Swoole\Client(SWOOLE_SOCK_TCP | SWOOLE_SSL, SWOOLE_SOCK_SYNC); //同步阻塞 - if (!$client->connect('127.0.0.1', $pm->getFreePort())) - { + if (!$client->connect('127.0.0.1', $pm->getFreePort())) { exit("connect failed\n"); } $client->send("hello world");