Fix http server coro memory leak (#5163)

* Fix memory leak * Fix segmentfault * optimize code and update version * optimize code and update version
swoole · Oct 13, 2023 · facc147 · facc147
1 parent b72d22a
commit facc147
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 27 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -1,7 +1,7 @@
 PROJECT(libswoole)
 
 ENABLE_LANGUAGE(ASM)
-set(SWOOLE_VERSION 5.1.0)
+set(SWOOLE_VERSION 5.1.1-DEV)
 
 set(CMAKE_CXX_STANDARD 11)
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -Wall -g")

diff --git a/ext-src/swoole_http_server_coro.cc b/ext-src/swoole_http_server_coro.cc
@@ -100,6 +100,9 @@ class HttpServer {
 
     ~HttpServer() {
         sw_free(upload_tmp_dir);
+        zval_ptr_dtor(&zcallbacks);
+        zval_ptr_dtor(&zclients);
+        delete socket;
     }
 
     void set_handler(std::string pattern, zval *zcallback, const zend_fcall_info_cache *fci_cache) {
@@ -256,8 +259,6 @@ static void php_swoole_http_server_coro_free_object(zend_object *object) {
     HttpServerObject *hsc = php_swoole_http_server_coro_fetch_object(object);
     if (hsc->server) {
         HttpServer *hs = hsc->server;
-        zval_ptr_dtor(&hs->zcallbacks);
-        zval_ptr_dtor(&hs->zclients);
         delete hs;
     }
     zend_object_std_dtor(&hsc->std);
@@ -564,15 +565,12 @@ static PHP_METHOD(swoole_http_server_coro, onAccept) {
         RETURN_FALSE;
     }
 #endif
-
-    std::string cid_str = std::to_string(co->get_cid());
-    zend::array_set(&hs->zclients, cid_str.c_str(), cid_str.length(), zconn);
-
-    auto addr = sock->get_ip();
-    zend_string *remote_addr = zend_string_init(addr, strlen(addr), 0);
+    Z_TRY_ADDREF_P(zconn);
+    zend_hash_index_add(Z_ARRVAL_P(&hs->zclients), co->get_cid(), zconn);
+    zend::Variable remote_addr = zend::Variable(sock->get_ip());
 
     while (true) {
-    _recv_request : {
+    _recv_request: {
         sock->get_socket()->recv_wait = 1;
         ssize_t retval = sock->recv(buffer->str + buffer->length, buffer->size - buffer->length);
         if (sw_unlikely(retval <= 0)) {
@@ -660,12 +658,11 @@ static PHP_METHOD(swoole_http_server_coro, onAccept) {
         zend::assign_zend_string_by_val(&ctx->request.zdata, buffer->pop(SW_BUFFER_SIZE_BIG), total_length);
 
         zval *zserver = ctx->request.zserver;
-        add_assoc_long(zserver, "server_port", hs->socket->get_bind_port());
-        add_assoc_long(zserver, "remote_port", (zend_long) sock->get_port());
-
-        zval tmp;
-        ZVAL_STR_COPY(&tmp, remote_addr);
-        zend_hash_str_add(Z_ARRVAL_P(zserver), ZEND_STRL("remote_addr"), &tmp);
+        http_server_add_server_array(
+            Z_ARRVAL_P(zserver), SW_ZSTR_KNOWN(SW_ZEND_STR_SERVER_PORT), hs->socket->get_bind_port());
+        http_server_add_server_array(Z_ARRVAL_P(zserver), SW_ZSTR_KNOWN(SW_ZEND_STR_REMOTE_PORT), sock->get_port());
+        http_server_add_server_array(Z_ARRVAL_P(zserver), SW_ZSTR_KNOWN(SW_ZEND_STR_REMOTE_ADDR), remote_addr.ptr());
+        remote_addr.add_ref();
 
         zend_fcall_info_cache *fci_cache = hs->get_handler(ctx);
         zval args[2] = {*ctx->request.zobject, *ctx->response.zobject};
@@ -700,23 +697,23 @@ static PHP_METHOD(swoole_http_server_coro, onAccept) {
         zval_dtor(ctx->request.zobject);
         zval_dtor(ctx->response.zobject);
     }
-    zend_string_release(remote_addr);
-    zend::array_unset(&hs->zclients, cid_str.c_str(), cid_str.length());
+    zend_hash_index_del(Z_ARRVAL_P(&hs->zclients), co->get_cid());
 }
 
 static PHP_METHOD(swoole_http_server_coro, shutdown) {
     HttpServer *hs = http_server_get_object(Z_OBJ_P(ZEND_THIS));
     hs->running = false;
     hs->socket->cancel(SW_EVENT_READ);
-    zend_hash_apply(Z_ARRVAL_P(&hs->zclients), [](zval *zconn) -> int {
+
+    zend_ulong index;
+    zval *zconn;
+    ZEND_HASH_FOREACH_NUM_KEY_VAL(Z_ARRVAL_P(&hs->zclients), index, zconn) {
         Socket *sock = php_swoole_get_socket(zconn);
         if (sock->get_socket()->recv_wait) {
             sock->cancel(SW_EVENT_READ);
-            return ZEND_HASH_APPLY_REMOVE;
-        } else {
-            return ZEND_HASH_APPLY_KEEP;
+            zend_hash_index_del(Z_ARRVAL_P(&hs->zclients), index);
         }
-    });
+    } ZEND_HASH_FOREACH_END();
 }
 
 static void http2_server_onRequest(Http2Session *session, Http2Stream *stream) {

diff --git a/include/swoole_version.h b/include/swoole_version.h
@@ -20,10 +20,10 @@
 
 #define SWOOLE_MAJOR_VERSION 5
 #define SWOOLE_MINOR_VERSION 1
-#define SWOOLE_RELEASE_VERSION 0
-#define SWOOLE_EXTRA_VERSION ""
-#define SWOOLE_VERSION "5.1.0"
-#define SWOOLE_VERSION_ID 50100
+#define SWOOLE_RELEASE_VERSION 1
+#define SWOOLE_EXTRA_VERSION "dev"
+#define SWOOLE_VERSION "5.1.1-dev"
+#define SWOOLE_VERSION_ID 50101
 #define SWOOLE_API_VERSION_ID 0x202208a
 
 #define SWOOLE_BUG_REPORT                                                                                              \