Skip to content

Commit

Permalink
net/tcp: use independent work to free the conn instance
Browse files Browse the repository at this point in the history
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
| apache#7  0x56577230 in netdriver_txavail_work (arg=0x5660bd80 <g_sim_dev>) at sim/up_netdriver.c:308
| apache#8  0x5655999e in work_thread (argc=2, argv=0xf3db5dd0) at wqueue/kwork_thread.c:178
| apache#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
| apache#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 <anchao@xiaomi.com>
  • Loading branch information
anchao authored and xiaoxiang781216 committed Sep 22, 2022
1 parent d35307d commit e46688c
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 5 deletions.
1 change: 1 addition & 0 deletions net/tcp/tcp.h
Original file line number Diff line number Diff line change
Expand Up @@ -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() */
Expand Down
17 changes: 12 additions & 5 deletions net/tcp/tcp_close.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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;
}
Expand Down
6 changes: 6 additions & 0 deletions net/tcp/tcp_conn.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit e46688c

Please sign in to comment.