diff options
-rwxr-xr-x | nuttx/ChangeLog | 6 | ||||
-rw-r--r-- | nuttx/net/socket/send.c | 7 | ||||
-rw-r--r-- | nuttx/net/tcp/tcp.h | 2 | ||||
-rw-r--r-- | nuttx/net/tcp/tcp_send_buffered.c | 91 | ||||
-rw-r--r-- | nuttx/net/tcp/tcp_wrbuffer.c | 5 |
5 files changed, 63 insertions, 48 deletions
diff --git a/nuttx/ChangeLog b/nuttx/ChangeLog index 3ea6bd3fc..e79c3a46e 100755 --- a/nuttx/ChangeLog +++ b/nuttx/ChangeLog @@ -9550,3 +9550,9 @@ there are non-waiting versions of the same interfaces (like iob_tryalloc()). The TCP read-ahead logic now uses only these non- waiting interfaces (2015-01-27). + * net/tcp/tcp_send_buffered.c and tcp_wrbuffer.c: Fix another deadlock + condition. tcp_write_buffer_alloc() calls sem_wait() with network + locked. That worked if CONFIG_NET_NOINTS was not defined because + interrupts are automatically restored when the wait happens. But + with CONFIG_NET_NOINTS=y, the wait blocks with the network locked -- + bad style and also can lead to a deadlock condition (2015-01-28). diff --git a/nuttx/net/socket/send.c b/nuttx/net/socket/send.c index d40705e59..db8fa2f1a 100644 --- a/nuttx/net/socket/send.c +++ b/nuttx/net/socket/send.c @@ -169,8 +169,13 @@ ssize_t psock_send(FAR struct socket *psock, FAR const void *buf, size_t len, default: { - ret = ERROR; + /* EDESTADDRREQ. Signifies that the socket is not connection-mode + * and no peer address is set. + */ + + ret = -EDESTADDRREQ; } + break; } return ret; diff --git a/nuttx/net/tcp/tcp.h b/nuttx/net/tcp/tcp.h index 010b00989..0f8146b70 100644 --- a/nuttx/net/tcp/tcp.h +++ b/nuttx/net/tcp/tcp.h @@ -1013,7 +1013,7 @@ void tcp_wrbuffer_initialize(void); * None * * Assumptions: - * Called from user logic with interrupts enabled. + * Called from user logic with the network locked. * ****************************************************************************/ diff --git a/nuttx/net/tcp/tcp_send_buffered.c b/nuttx/net/tcp/tcp_send_buffered.c index 43f80a856..ca8dddc4a 100644 --- a/nuttx/net/tcp/tcp_send_buffered.c +++ b/nuttx/net/tcp/tcp_send_buffered.c @@ -918,6 +918,7 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf, size_t len) { FAR struct tcp_conn_s *conn; + FAR struct tcp_wrbuffer_s *wrb; net_lock_t save; ssize_t result = 0; int err; @@ -940,6 +941,7 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf, /* Make sure that the IP address mapping is in the ARP table */ conn = (FAR struct tcp_conn_s *)psock->s_conn; + #ifdef CONFIG_NET_ARP_SEND ret = arp_send(conn->u.ipv4.raddr); if (ret < 0) @@ -958,10 +960,23 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf, psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_SEND); - save = net_lock(); - if (len > 0) { + /* Allocate a write buffer. Careful, the network will be momentarily + * unlocked here. + */ + + save = net_lock(); + wrb = tcp_wrbuffer_alloc(); + if (!wrb) + { + /* A buffer allocation error occurred */ + + ndbg("ERROR: Failed to allocate write buffer\n"); + err = ENOMEM; + goto errout_with_lock; + } + /* Allocate resources to receive a callback */ if (!psock->s_sndcb) @@ -976,61 +991,43 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf, /* A buffer allocation error occurred */ ndbg("ERROR: Failed to allocate callback\n"); - result = -ENOMEM; + err = ENOMEM; + goto errout_with_wrb; } - else - { - FAR struct tcp_wrbuffer_s *wrb; - /* Set up the callback in the connection */ + /* Set up the callback in the connection */ - psock->s_sndcb->flags = (TCP_ACKDATA | TCP_REXMIT | TCP_POLL | - TCP_CLOSE | TCP_ABORT | TCP_TIMEDOUT); - psock->s_sndcb->priv = (void*)psock; - psock->s_sndcb->event = psock_send_interrupt; + psock->s_sndcb->flags = (TCP_ACKDATA | TCP_REXMIT | TCP_POLL | + TCP_CLOSE | TCP_ABORT | TCP_TIMEDOUT); + psock->s_sndcb->priv = (void*)psock; + psock->s_sndcb->event = psock_send_interrupt; - /* Allocate an write buffer */ + /* Initialize the write buffer */ - wrb = tcp_wrbuffer_alloc(); - if (wrb) - { - /* Initialize the write buffer */ - - WRB_SEQNO(wrb) = (unsigned)-1; - WRB_NRTX(wrb) = 0; - WRB_COPYIN(wrb, (FAR uint8_t *)buf, len); + WRB_SEQNO(wrb) = (unsigned)-1; + WRB_NRTX(wrb) = 0; + WRB_COPYIN(wrb, (FAR uint8_t *)buf, len); - /* Dump I/O buffer chain */ + /* Dump I/O buffer chain */ - WRB_DUMP("I/O buffer chain", wrb, WRB_PKTLEN(wrb), 0); + WRB_DUMP("I/O buffer chain", wrb, WRB_PKTLEN(wrb), 0); - /* psock_send_interrupt() will send data in FIFO order from the - * conn->write_q - */ - - sq_addlast(&wrb->wb_node, &conn->write_q); - nvdbg("Queued WRB=%p pktlen=%u write_q(%p,%p)\n", - wrb, WRB_PKTLEN(wrb), - conn->write_q.head, conn->write_q.tail); + /* psock_send_interrupt() will send data in FIFO order from the + * conn->write_q + */ - /* Notify the device driver of the availability of TX data */ + sq_addlast(&wrb->wb_node, &conn->write_q); + nvdbg("Queued WRB=%p pktlen=%u write_q(%p,%p)\n", + wrb, WRB_PKTLEN(wrb), + conn->write_q.head, conn->write_q.tail); - send_txnotify(psock, conn); - result = len; - } + /* Notify the device driver of the availability of TX data */ - /* A buffer allocation error occurred */ - - else - { - ndbg("ERROR: Failed to allocate write buffer\n"); - result = -ENOMEM; - } - } + send_txnotify(psock, conn); + net_unlock(save); + result = len; } - net_unlock(save); - /* Set the socket state to idle */ psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE); @@ -1059,6 +1056,12 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf, return result; +errout_with_wrb: + tcp_wrbuffer_release(wrb); + +errout_with_lock: + net_unlock(save); + errout: set_errno(err); return ERROR; diff --git a/nuttx/net/tcp/tcp_wrbuffer.c b/nuttx/net/tcp/tcp_wrbuffer.c index 162d30ad6..3bdb8b2ba 100644 --- a/nuttx/net/tcp/tcp_wrbuffer.c +++ b/nuttx/net/tcp/tcp_wrbuffer.c @@ -54,6 +54,7 @@ #include <assert.h> #include <debug.h> +#include <nuttx/net/net.h> #include <nuttx/net/iob.h> #include "tcp/tcp.h" @@ -132,7 +133,7 @@ void tcp_wrbuffer_initialize(void) * None * * Assumptions: - * Called from user logic with interrupts enabled. + * Called from user logic with the network locked. * ****************************************************************************/ @@ -148,7 +149,7 @@ FAR struct tcp_wrbuffer_s *tcp_wrbuffer_alloc(void) * buffer */ - DEBUGVERIFY(sem_wait(&g_wrbuffer.sem)); + DEBUGVERIFY(net_lockedwait(&g_wrbuffer.sem)); /* Now, we are guaranteed to have a write buffer structure reserved * for us in the free list. |