You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2022/09/22 15:33:07 UTC
[incubator-nuttx] branch master updated: net/tcp: use independent work to free the conn instance
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new e46688c1ee net/tcp: use independent work to free the conn instance
e46688c1ee is described below
commit e46688c1ee08e4bdc76184d8a6c640a22ab4f42c
Author: chao an <an...@xiaomi.com>
AuthorDate: Tue Aug 30 11:04:31 2022 +0800
net/tcp: use independent work to free the conn instance
I noticed that the conn instance will leak during stress test,
The close work queued from tcp_close_eventhandler() will be canceled
by tcp_timer() immediately:
Breakpoint 1, tcp_close_eventhandler (dev=0x565cd338 <up_irq_restore+108>, pvpriv=0x5655e6ff <getpid+12>, flags=0) at tcp/tcp_close.c:71
(gdb) bt
| #0 tcp_close_eventhandler (dev=0x565cd338 <up_irq_restore+108>, pvpriv=0x5655e6ff <getpid+12>, flags=0) at tcp/tcp_close.c:71
| #1 0x5658bf1e in devif_conn_event (dev=0x5660bd80 <g_sim_dev>, flags=512, list=0x5660d558 <g_cbprealloc+312>) at devif/devif_callback.c:508
| #2 0x5658a219 in tcp_callback (dev=0x5660bd80 <g_sim_dev>, conn=0x5660c4a0 <g_tcp_connections>, flags=512) at tcp/tcp_callback.c:167
| #3 0x56589253 in tcp_timer (dev=0x5660bd80 <g_sim_dev>, conn=0x5660c4a0 <g_tcp_connections>) at tcp/tcp_timer.c:378
| #4 0x5658dd47 in tcp_poll (dev=0x5660bd80 <g_sim_dev>, conn=0x5660c4a0 <g_tcp_connections>) at tcp/tcp_devpoll.c:95
| #5 0x5658b95f in devif_poll_tcp_connections (dev=0x5660bd80 <g_sim_dev>, callback=0x565770f2 <netdriver_txpoll>) at devif/devif_poll.c:601
| #6 0x5658b9ea in devif_poll (dev=0x5660bd80 <g_sim_dev>, callback=0x565770f2 <netdriver_txpoll>) at devif/devif_poll.c:722
| #7 0x56577230 in netdriver_txavail_work (arg=0x5660bd80 <g_sim_dev>) at sim/up_netdriver.c:308
| #8 0x5655999e in work_thread (argc=2, argv=0xf3db5dd0) at wqueue/kwork_thread.c:178
| #9 0x5655983f in nxtask_start () at task/task_start.c:129
(gdb) c
Continuing.
Breakpoint 2, tcp_update_timer (conn=0x5660c4a0 <g_tcp_connections>) at tcp/tcp_timer.c:178
(gdb) bt
| #0 tcp_update_timer (conn=0x5660c4a0 <g_tcp_connections>) at tcp/tcp_timer.c:178
| #1 0x5658952a in tcp_timer (dev=0x5660bd80 <g_sim_dev>, conn=0x5660c4a0 <g_tcp_connections>) at tcp/tcp_timer.c:708
| #2 0x5658dd47 in tcp_poll (dev=0x5660bd80 <g_sim_dev>, conn=0x5660c4a0 <g_tcp_connections>) at tcp/tcp_devpoll.c:95
| #3 0x5658b95f in devif_poll_tcp_connections (dev=0x5660bd80 <g_sim_dev>, callback=0x565770f2 <netdriver_txpoll>) at devif/devif_poll.c:601
| #4 0x5658b9ea in devif_poll (dev=0x5660bd80 <g_sim_dev>, callback=0x565770f2 <netdriver_txpoll>) at devif/devif_poll.c:722
| #5 0x56577230 in netdriver_txavail_work (arg=0x5660bd80 <g_sim_dev>) at sim/up_netdriver.c:308
| #6 0x5655999e in work_thread (argc=2, argv=0xf3db5dd0) at wqueue/kwork_thread.c:178
| #7 0x5655983f in nxtask_start () at task/task_start.c:129
Since a separate work will add 24 bytes to each conn instance,
but in order to support the feature of asynchronous close(),
I can not find a better way than adding a separate work,
for resource constraints, I recommend the developers to enable
CONFIG_NET_ALLOC_CONNS, which will reduce the ram usage.
Signed-off-by: chao an <an...@xiaomi.com>
---
net/tcp/tcp.h | 1 +
net/tcp/tcp_close.c | 17 ++++++++++++-----
net/tcp/tcp_conn.c | 6 ++++++
3 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/net/tcp/tcp.h b/net/tcp/tcp.h
index 8c3068768f..9647b92bca 100644
--- a/net/tcp/tcp.h
+++ b/net/tcp/tcp.h
@@ -304,6 +304,7 @@ struct tcp_conn_s
/* Reference to TCP close callback instance */
FAR struct devif_callback_s *clscb;
+ struct work_s clswork;
#if defined(CONFIG_NET_TCP_WRITE_BUFFERS)
/* Callback instance for TCP send() */
diff --git a/net/tcp/tcp_close.c b/net/tcp/tcp_close.c
index 4a98c9833e..86fc22a6e9 100644
--- a/net/tcp/tcp_close.c
+++ b/net/tcp/tcp_close.c
@@ -53,10 +53,13 @@ static void tcp_close_work(FAR void *param)
net_lock();
- /* Stop the network monitor for all sockets */
+ if (conn && conn->crefs == 0)
+ {
+ /* Stop the network monitor for all sockets */
- tcp_stop_monitor(conn, TCP_CLOSE);
- tcp_free(conn);
+ tcp_stop_monitor(conn, TCP_CLOSE);
+ tcp_free(conn);
+ }
net_unlock();
}
@@ -175,11 +178,15 @@ static uint16_t tcp_close_eventhandler(FAR struct net_driver_s *dev,
return flags;
end_wait:
- tcp_callback_free(conn, conn->clscb);
+ if (conn->clscb != NULL)
+ {
+ tcp_callback_free(conn, conn->clscb);
+ conn->clscb = NULL;
+ }
/* Free network resources */
- work_queue(LPWORK, &conn->work, tcp_close_work, conn, 0);
+ work_queue(LPWORK, &conn->clswork, tcp_close_work, conn, 0);
return flags;
}
diff --git a/net/tcp/tcp_conn.c b/net/tcp/tcp_conn.c
index 2532eb8ab1..4c3e2990b5 100644
--- a/net/tcp/tcp_conn.c
+++ b/net/tcp/tcp_conn.c
@@ -769,6 +769,12 @@ void tcp_free(FAR struct tcp_conn_s *conn)
DEBUGASSERT(conn->crefs == 0);
+ /* Cancel close work */
+
+ work_cancel(LPWORK, &conn->clswork);
+
+ /* Cancel tcp timer */
+
tcp_stop_timer(conn);
/* Free remaining callbacks, actually there should be only the send