You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by ac...@apache.org on 2022/02/26 14:48:13 UTC
[incubator-nuttx] branch master updated: tcp: added debug asserts and logging to investigate the rare (conn->dev == NULL) bug in callback handlers
This is an automated email from the ASF dual-hosted git repository.
acassis 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 404ceff tcp: added debug asserts and logging to investigate the rare (conn->dev == NULL) bug in callback handlers
404ceff is described below
commit 404ceffae2021d8ccf61f620f2f7cc6711f135c2
Author: Alexander Lunev <al...@mail.ru>
AuthorDate: Sat Jan 29 21:13:22 2022 +0300
tcp: added debug asserts and logging to investigate the rare (conn->dev == NULL) bug in callback handlers
---
net/inet/inet_sockif.c | 6 ++++++
net/tcp/Make.defs | 2 +-
net/tcp/tcp.h | 20 +++++++++++++++++
net/tcp/{tcp_wrbuffer_dump.c => tcp_dump.c} | 33 ++++++++++++++++++++++++++++-
net/tcp/tcp_send_buffered.c | 27 +++++++++++++++++++++++
net/tcp/tcp_send_unbuffered.c | 13 ++++++++++++
net/tcp/tcp_sendfile.c | 13 ++++++++++++
7 files changed, 112 insertions(+), 2 deletions(-)
diff --git a/net/inet/inet_sockif.c b/net/inet/inet_sockif.c
index 8858c66..061e6a6 100644
--- a/net/inet/inet_sockif.c
+++ b/net/inet/inet_sockif.c
@@ -157,6 +157,12 @@ static int inet_tcp_alloc(FAR struct socket *psock)
DEBUGASSERT(conn->crefs == 0);
conn->crefs = 1;
+ /* It is expected the socket has not yet been associated with
+ * any other connection.
+ */
+
+ DEBUGASSERT(psock->s_conn == NULL);
+
/* Save the pre-allocated connection in the socket structure */
psock->s_conn = conn;
diff --git a/net/tcp/Make.defs b/net/tcp/Make.defs
index 702c35e..e022836 100644
--- a/net/tcp/Make.defs
+++ b/net/tcp/Make.defs
@@ -60,7 +60,7 @@ NET_CSRCS += tcp_recvwindow.c tcp_netpoll.c tcp_ioctl.c
ifeq ($(CONFIG_NET_TCP_WRITE_BUFFERS),y)
NET_CSRCS += tcp_wrbuffer.c
ifeq ($(CONFIG_DEBUG_FEATURES),y)
-NET_CSRCS += tcp_wrbuffer_dump.c
+NET_CSRCS += tcp_dump.c
endif
endif
diff --git a/net/tcp/tcp.h b/net/tcp/tcp.h
index 030a52b..d5ec88c 100644
--- a/net/tcp/tcp.h
+++ b/net/tcp/tcp.h
@@ -311,6 +311,10 @@ struct tcp_conn_s
/* Callback instance for TCP send() */
FAR struct devif_callback_s *sndcb;
+
+#ifdef CONFIG_DEBUG_ASSERTIONS
+ int sndcb_alloc_cnt; /* The callback allocation counter */
+#endif
#endif
/* accept() is called when the TCP logic has created a connection
@@ -1643,6 +1647,22 @@ int tcp_wrbuffer_test(void);
#endif /* CONFIG_NET_TCP_WRITE_BUFFERS */
/****************************************************************************
+ * Name: tcp_event_handler_dump
+ *
+ * Description:
+ * Dump the TCP event handler related variables
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_DEBUG_FEATURES
+void tcp_event_handler_dump(FAR struct net_driver_s *dev,
+ FAR void *pvconn,
+ FAR void *pvpriv,
+ uint16_t flags,
+ FAR struct tcp_conn_s *conn);
+#endif
+
+/****************************************************************************
* Name: tcp_wrbuffer_dump
*
* Description:
diff --git a/net/tcp/tcp_wrbuffer_dump.c b/net/tcp/tcp_dump.c
similarity index 65%
rename from net/tcp/tcp_wrbuffer_dump.c
rename to net/tcp/tcp_dump.c
index c0980e2..4e5f082 100644
--- a/net/tcp/tcp_wrbuffer_dump.c
+++ b/net/tcp/tcp_dump.c
@@ -1,5 +1,5 @@
/****************************************************************************
- * net/tcp/tcp_wrbuffer_dump.c
+ * net/tcp/tcp_dump.c
*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
@@ -39,6 +39,33 @@
****************************************************************************/
/****************************************************************************
+ * Name: tcp_event_handler_dump
+ *
+ * Description:
+ * Dump the TCP event handler related variables
+ *
+ ****************************************************************************/
+
+void tcp_event_handler_dump(FAR struct net_driver_s *dev,
+ FAR void *pvconn,
+ FAR void *pvpriv,
+ uint16_t flags,
+ FAR struct tcp_conn_s *conn)
+{
+ nerr("ERROR: conn->dev == NULL or pvconn != conn:"
+ " dev=%p pvconn=%p pvpriv=%p flags=0x%04x"
+ " conn->dev=%p conn->flags=0x%04x tcpstateflags=0x%02x crefs=%d"
+ " isn=%" PRIu32 " sndseq=%" PRIu32
+ " tx_unacked=%" PRId32 " sent=%" PRId32
+ " conn=%p s_flags=0x%02x\n",
+ dev, pvconn, pvpriv, flags,
+ conn->dev, conn->flags, conn->tcpstateflags, conn->crefs,
+ conn->isn, tcp_getsequence(conn->sndseq),
+ (uint32_t)conn->tx_unacked, conn->sent,
+ conn, conn->sconn.s_flags);
+}
+
+/****************************************************************************
* Name: tcp_wrbuffer_dump
*
* Description:
@@ -46,6 +73,8 @@
*
****************************************************************************/
+#ifdef CONFIG_NET_TCP_WRITE_BUFFERS
+
void tcp_wrbuffer_dump(FAR const char *msg, FAR struct tcp_wrbuffer_s *wrb,
unsigned int len, unsigned int offset)
{
@@ -54,4 +83,6 @@ void tcp_wrbuffer_dump(FAR const char *msg, FAR struct tcp_wrbuffer_s *wrb,
iob_dump("I/O Buffer Chain", TCP_WBIOB(wrb), len, offset);
}
+#endif
+
#endif /* CONFIG_DEBUG_FEATURES */
diff --git a/net/tcp/tcp_send_buffered.c b/net/tcp/tcp_send_buffered.c
index 82f8b74..cb5c62f 100644
--- a/net/tcp/tcp_send_buffered.c
+++ b/net/tcp/tcp_send_buffered.c
@@ -371,6 +371,19 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev,
DEBUGASSERT(conn != NULL);
+#ifdef CONFIG_DEBUG_FEATURES
+ if (conn->dev == NULL || (pvconn != conn && pvconn != NULL))
+ {
+ tcp_event_handler_dump(dev, pvconn, pvpriv, flags, conn);
+ }
+#endif
+
+ /* If pvconn is not NULL, make sure that pvconn refers to the same
+ * connection as the socket is bound to.
+ */
+
+ DEBUGASSERT(pvconn == conn || pvconn == NULL);
+
/* The TCP socket is connected and, hence, should be bound to a device.
* Make sure that the polling device is the one that we are bound to.
*/
@@ -1171,6 +1184,20 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf,
if (conn->sndcb == NULL)
{
conn->sndcb = tcp_callback_alloc(conn);
+
+#ifdef CONFIG_DEBUG_ASSERTIONS
+ if (conn->sndcb != NULL)
+ {
+ conn->sndcb_alloc_cnt++;
+
+ /* The callback is allowed to be allocated only once.
+ * This is to catch a potential re-allocation after
+ * conn->sndcb was set to NULL.
+ */
+
+ DEBUGASSERT(conn->sndcb_alloc_cnt == 1);
+ }
+#endif
}
/* Test if the callback has been allocated */
diff --git a/net/tcp/tcp_send_unbuffered.c b/net/tcp/tcp_send_unbuffered.c
index f7dafa3..3c8d478 100644
--- a/net/tcp/tcp_send_unbuffered.c
+++ b/net/tcp/tcp_send_unbuffered.c
@@ -196,6 +196,19 @@ static uint16_t tcpsend_eventhandler(FAR struct net_driver_s *dev,
conn = psock->s_conn;
DEBUGASSERT(conn != NULL);
+#ifdef CONFIG_DEBUG_NET_ERROR
+ if (conn->dev == NULL || (pvconn != conn && pvconn != NULL))
+ {
+ tcp_event_handler_dump(dev, pvconn, pvpriv, flags, conn);
+ }
+#endif
+
+ /* If pvconn is not NULL, make sure that pvconn refers to the same
+ * connection as the socket is bound to.
+ */
+
+ DEBUGASSERT(pvconn == conn || pvconn == NULL);
+
/* The TCP socket is connected and, hence, should be bound to a device.
* Make sure that the polling device is the one that we are bound to.
*/
diff --git a/net/tcp/tcp_sendfile.c b/net/tcp/tcp_sendfile.c
index 535dbcc..509301f 100644
--- a/net/tcp/tcp_sendfile.c
+++ b/net/tcp/tcp_sendfile.c
@@ -150,6 +150,19 @@ static uint16_t sendfile_eventhandler(FAR struct net_driver_s *dev,
conn = psock->s_conn;
DEBUGASSERT(conn != NULL);
+#ifdef CONFIG_DEBUG_NET_ERROR
+ if (conn->dev == NULL || (pvconn != conn && pvconn != NULL))
+ {
+ tcp_event_handler_dump(dev, pvconn, pvpriv, flags, conn);
+ }
+#endif
+
+ /* If pvconn is not NULL, make sure that pvconn refers to the same
+ * connection as the socket is bound to.
+ */
+
+ DEBUGASSERT(pvconn == conn || pvconn == NULL);
+
/* The TCP socket is connected and, hence, should be bound to a device.
* Make sure that the polling device is the own that we are bound to.
*/