From a6cc2d5aee53099d384f545ebf5f384c0a545f38 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 15 Dec 2014 23:35:13 +0000 Subject: [PATCH] agent: Handle EWOULDBLOCK when transmitting pseudo-TCP segments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pseudo-TCP code previously didn’t handle EAGAIN or EWOULDBLOCK errors from the low-level NiceSocket code. This caused pseudo-TCP connections to be dropped if the transmitting socket ever filled up, which could cause problems on high bandwidth connections. Work around the issue by handling EWOULDBLOCK in the NiceSocket-to-pseudo-TCP converter in agent.c. A full fix for the problem would involve plumbing the EWOULDBLOCK error all the way up into the pseudo-TCP implementation, so that blocked segments are requeued without the TCP window changing. That is hard, and time is short, so instead spin in a tight loop, attempting to send the packet until successful, or until the iteration limit is reached. This isn’t great, but should work fine under the assumption that EWOULDBLOCK is almost always transient, and that if it isn’t, the connection’s probably got some huge problems and will time out anyway. Spotted and debugged by Radosław Kołodziejczyk . --- agent/agent.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/agent/agent.c b/agent/agent.c index a2fd46d..10eac73 100644 --- a/agent/agent.c +++ b/agent/agent.c @@ -1729,6 +1729,9 @@ pseudo_tcp_socket_closed (PseudoTcpSocket *sock, guint32 err, priv_pseudo_tcp_error (agent, stream, component); } +/* Number of retries to transmit a segment when receiving EWOULDBLOCK. After + * this limit is hit, transmission fails and the socket is closed. */ +#define RETRY_LIMIT 1000 static PseudoTcpWriteResult pseudo_tcp_socket_write_packet (PseudoTcpSocket *psocket, @@ -1739,6 +1742,8 @@ pseudo_tcp_socket_write_packet (PseudoTcpSocket *psocket, if (component->selected_pair.local != NULL) { NiceSocket *sock; NiceAddress *addr; + guint i; + gssize sent; sock = component->selected_pair.local->sockptr; addr = &component->selected_pair.remote->addr; @@ -1754,8 +1759,24 @@ pseudo_tcp_socket_write_packet (PseudoTcpSocket *psocket, nice_address_get_port (addr)); } - if (nice_socket_send (sock, addr, len, buffer)) + /* Retry transmission until we don’t get EWOULDBLOCK. + * + * FIXME: Ideally, this would be handled within the pseudo-TCP stack, + * re-queuing the segment if pseudo_tcp_socket_write_packet() returns + * EWOULDBLOCK. However, that would require a lot of reworking of the + * pseudo-TCP code to avoid adjusting the state machine until the retry + * limit is hit. For now, under the assumption that EWOULDBLOCK is very + * transient, just spin in a loop until it works. */ + for (i = 0, sent = 0; i < RETRY_LIMIT && len != 0 && sent == 0; i++) { + sent = nice_socket_send (sock, addr, len, buffer); + } + + if (sent > 0 || len == 0) { return WR_SUCCESS; + } + + nice_debug ("%s: WARNING: Failed to send pseudo-TCP packet due to repeated " + "blocked socket. %u attempts to transmit %u bytes.", G_STRFUNC, i, len); } else { nice_debug ("%s: WARNING: Failed to send pseudo-TCP packet from agent %p " "as no pair has been selected yet.", G_STRFUNC, component->agent); -- 1.9.3