proxy client: привести lwIP TCP close к лучшим практикам

handle_close: tcp_abort→tcp_close + полное обнуление коллбэков (graceful FIN вместо RST) handle_error: отделён от handle_close, свой cleanup с tcp_abort (RST — нештатная ситуация) poll_cb/destroy: полное обнуление коллбэков перед tcp_abort (tcp_arg/recv/sent/err/poll) handle_fin: исправлен лог (shutdown write, не read) tx_waiter_cb: проверка pc->pcb перед tcp_recved
4 months ago · 7a35f4c386
2 changed files with 148 additions and 7 deletions
--- a/src/proxy/lwip.txt
+++ b/src/proxy/lwip.txt
@ -0,0 +1,106 @@
+Как узнать, что TCP полностью завершился?
+
+В raw API прямого callback'а вида:
+
+on_connection_closed()
+
+нет.
+
+Обычно используют собственное состояние:
+
+struct conn {
+    bool closing;
+};
+
+и после успешного:
+
+tcp_close(pcb);
+conn->closing = true;
+
+считают соединение закрытым для приложения.
+
+Сам TCP завершится позже внутри lwIP.
+
+А вызовется ли err callback?
+
+Обычно нет.
+
+tcp_err()
+
+вызывается при ненормальном завершении:
+
+RST
+abort
+потеря PCB
+сетевые ошибки
+
+но не при штатном FIN/ACK закрытии.
+
+Нужно ли освобождать память после tcp_close()?
+
+Сам pcb освобождать нельзя:
+
+tcp_close(pcb);
+
+/* НЕЛЬЗЯ */
+mem_free(pcb);
+
+lwIP сделает это самостоятельно.
+
+Но ваши данные (arg) нужно освободить самостоятельно.
+
+Обычно так:
+
+struct my_conn *c = arg;
+
+tcp_arg(pcb, NULL);
+
+free(c);
+
+tcp_close(pcb);
+
+или
+
+free(c);
+tcp_close(pcb);
+pcb = NULL;
+
+если после этого код больше не обращается к c.
+
+Безопасная практика
+
+После успешного tcp_close():
+
+if (tcp_close(pcb) == ERR_OK) {
+    pcb = NULL;
+}
+
+и считать объект недействительным.
+
+Потому что lwIP может уничтожить его в любой момент по мере завершения TCP state machine.
+
+Типичный шаблон
+static void close_conn(struct my_conn *c)
+{
+    err_t err;
+
+    err = tcp_close(c->pcb);
+
+    if (err == ERR_OK) {
+        c->pcb = NULL;
+        free(c);
+    } else {
+        /* ERR_MEM -> попробуем позже */
+    }
+}
+
+Но если free(c) делается сразу, нужно быть уверенным, что никакие callback'и lwIP больше не используют arg. Часто перед закрытием сначала снимают callback'и:
+
+tcp_arg(pcb, NULL);
+tcp_recv(pcb, NULL);
+tcp_sent(pcb, NULL);
+tcp_err(pcb, NULL);
+
+а затем вызывают tcp_close().
+
+Это особенно важно в сложных приложениях с собственными структурами соединений.
--- a/src/proxy/tcp_proxy_client.c
+++ b/src/proxy/tcp_proxy_client.c
@ -253,7 +253,7 @@ static void tcp_proxy_client_tx_waiter_cb(struct ll_queue* q, void* arg) {
    if (!pc || !pc->tx_buf) return;
    int ret = tcp_proxy_client_send_data(pc, pc->tx_buf, pc->tx_len);
    if (ret == 0) {
-        tcp_recved(pc->pcb, pc->tx_len);
+        if (pc->pcb) tcp_recved(pc->pcb, pc->tx_len);
        u_free(pc->tx_buf); pc->tx_buf = NULL; pc->tx_len = 0;
        if (pc->fin_local && !pc->close_sent && !pc->close_pending) {
            if (pc->fin_remote || pc->rem_closed)
@ -336,7 +336,15 @@ static err_t tcp_proxy_client_poll_cb(void *arg, struct tcp_pcb *pcb) {

    if (pc->error) {
        DEBUG_ERROR(DEBUG_CATEGORY_SOCKET, "PROXY CLEANUP error sid=%08x", pc->stream_id);
-        if (pc->pcb) { tcp_arg(pc->pcb, NULL); tcp_abort(pc->pcb); pc->pcb = NULL; }
+        if (pc->pcb) {
+            tcp_arg(pc->pcb, NULL);
+            tcp_recv(pc->pcb, NULL);
+            tcp_sent(pc->pcb, NULL);
+            tcp_err(pc->pcb, NULL);
+            tcp_poll(pc->pcb, NULL, 0);
+            tcp_abort(pc->pcb);
+            pc->pcb = NULL;
+        }
        tcp_proxy_client_conn_free(pc);
        return LERR_OK;
    }
@ -450,11 +458,19 @@ static void tcp_proxy_client_handle_data(struct tcp_proxy_client* p, struct ETCP
 static void tcp_proxy_client_handle_close(struct tcp_proxy_client* p, uint32_t stream_id) {
    struct tcp_proxy_client_conn* pc = tcp_proxy_client_find_conn(p, stream_id);
    if (!pc) { DEBUG_WARN(DEBUG_CATEGORY_SOCKET, "PROXY CLOSE sid=%08x — no conn, dropping", stream_id); return; }
-    DEBUG_INFO(DEBUG_CATEGORY_SOCKET, "PROXY REM_CLOSED sid=%08x fin_local=%d", stream_id, pc->fin_local);
+    DEBUG_INFO(DEBUG_CATEGORY_SOCKET, "PROXY REM_CLOSED sid=%08x fin_local=%d pcb_state=%u", stream_id, pc->fin_local, pc->pcb ? pc->pcb->state : 0);
    pc->rem_closed = 1;
    if (pc->tx_buf) { u_free(pc->tx_buf); pc->tx_buf = NULL; pc->tx_len = 0; }
    etcp_router_waiter_cancel(pc->proxy->inst, pc->proxy->via_node_id, &pc->tx_waiter);
-    if (pc->pcb) { tcp_arg(pc->pcb, NULL); tcp_abort(pc->pcb); pc->pcb = NULL; }
+    if (pc->pcb) {
+        tcp_arg(pc->pcb, NULL);
+        tcp_recv(pc->pcb, NULL);
+        tcp_sent(pc->pcb, NULL);
+        tcp_err(pc->pcb, NULL);
+        tcp_poll(pc->pcb, NULL, 0);
+        tcp_close(pc->pcb);
+        pc->pcb = NULL;
+    }
    tcp_proxy_client_conn_free(pc);
 }

@ -462,13 +478,25 @@ static void tcp_proxy_client_handle_error(struct tcp_proxy_client* p, uint32_t s
    struct tcp_proxy_client_conn* pc = tcp_proxy_client_find_conn(p, stream_id);
    if (!pc) { DEBUG_INFO(DEBUG_CATEGORY_SOCKET, "PROXY ERROR sid=%08x — no conn, dropping", stream_id); return; }
    DEBUG_ERROR(DEBUG_CATEGORY_SOCKET, "PROXY ERROR from exit sid=%08x fin_local=%d", stream_id, pc->fin_local);
-    tcp_proxy_client_handle_close(p, stream_id);
+    pc->rem_closed = 1;
+    if (pc->tx_buf) { u_free(pc->tx_buf); pc->tx_buf = NULL; pc->tx_len = 0; }
+    etcp_router_waiter_cancel(pc->proxy->inst, pc->proxy->via_node_id, &pc->tx_waiter);
+    if (pc->pcb) {
+        tcp_arg(pc->pcb, NULL);
+        tcp_recv(pc->pcb, NULL);
+        tcp_sent(pc->pcb, NULL);
+        tcp_err(pc->pcb, NULL);
+        tcp_poll(pc->pcb, NULL, 0);
+        tcp_abort(pc->pcb);
+        pc->pcb = NULL;
+    }
+    tcp_proxy_client_conn_free(pc);
 }

 static void tcp_proxy_client_handle_fin(struct tcp_proxy_client* p, uint32_t stream_id) {
    struct tcp_proxy_client_conn* pc = tcp_proxy_client_find_conn(p, stream_id);
    if (!pc || !pc->pcb) return;
-    DEBUG_INFO(DEBUG_CATEGORY_SOCKET, "PROXY FIN FROM exit sid=%08x — shutdown read", stream_id);
+    DEBUG_INFO(DEBUG_CATEGORY_SOCKET, "PROXY FIN FROM exit sid=%08x — shutdown write (send FIN to local)", stream_id);
    pc->fin_remote = 1;
    if (pc->pcb->state != TIME_WAIT && pc->pcb->state != CLOSED)
        tcp_shutdown(pc->pcb, 0, 1);
@ -584,7 +612,14 @@ void tcp_proxy_client_destroy(struct tcp_proxy_client* p) {
    struct tcp_proxy_client_conn* pc = p->conns;
    while (pc) {
        struct tcp_proxy_client_conn* next = pc->next;
-        if (pc->pcb && pc->pcb->state != CLOSED) { tcp_arg(pc->pcb, NULL); tcp_abort(pc->pcb); }
+        if (pc->pcb && pc->pcb->state != CLOSED) {
+            tcp_arg(pc->pcb, NULL);
+            tcp_recv(pc->pcb, NULL);
+            tcp_sent(pc->pcb, NULL);
+            tcp_err(pc->pcb, NULL);
+            tcp_poll(pc->pcb, NULL, 0);
+            tcp_abort(pc->pcb);
+        }
        pc->pcb = NULL;
        if (pc->to_lwip) {
            struct ll_entry *e;