Optimize ssl context code, fix tests, fix memory leak

swoole · Nov 5, 2024 · 050293e · 050293e
1 parent 2dd90db
commit 050293e
Show file tree

Hide file tree

Showing 8 changed files with 36 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<std::list<swoole::Callback> *>(SwooleG.hooks[i]);
+            delete hooks;
+        }
+    }
     swoole_signal_clear();
     SwooleG = {};
 }

diff --git 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
@@ -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");