j3d1/tinc
1
0
Fork 0
Code Issues 1 Pull requests Releases Wiki Activity

Fix connection event error handling.

Browse source 
Commit 86a99c6b99 changed the way we
handle connection events to protect against spurious event loop
callbacks. Unfortunately, it turns out that calling connect() twice on
the same socket results in different behaviors depending on the platform
(even though it seems well defined in POSIX). On Windows this resulted
in the connection handling code being unable to react to connection
errors (such as connection refused), always hitting the timeout; on
Linux this resulted in spurious error messages about connect() returning
success.

In POSIX and on Linux, using connect() on a socket where the previous
attempt failed will attempt to connect again, resulting in unnecessary
network activity. Using getsockopt(SO_ERROR) before connect() solves
that, but introduces a race condition if a connection failure happens
between the two calls.

For this reason, this commit switches from connect() to a zero-sized
send() call, which is more consistent (though not completely, see the
truth table in the comments) and simpler to use for that purpose. Note
that Windows explictly support empty send() calls; POSIX says nothing
on the subject, but testing shows it works at least on Linux.

(Surprisingly enough, Windows seems more POSIX-compliant than Linux on
this one!)
This commit is contained in:
Etienne Dechamps 2014-06-28 11:13:29 +01:00
parent 86a99c6b99
commit cc284e7c5d
2 changed files with 28 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
src/net_socket.c
View file

@ -401,30 +401,38 @@ static void handle_meta_io(void *data, int flags) {
connection_t *c = data;
if(c->status.connecting) {
/* The event loop does not protect against spurious events. Verify that we are actually connected. */
if (connect(c->socket, &c->address.sa, sizeof(c->address)) == 0)
logger(DEBUG_CONNECTIONS, LOG_DEBUG, "Error while connecting to %s (%s): redundant connect() unexpectedly succeeded", c->name, c->hostname);
else if (!sockisconn(sockerrno)) {
if (!sockalready(sockerrno)) {
logger(DEBUG_CONNECTIONS, LOG_DEBUG, "Error while checking connection status for %s (%s): %s", c->name, c->hostname, sockstrerror(sockerrno));
/*
The event loop does not protect against spurious events. Verify that we are actually connected
by issuing an empty send() call.
Note that the behavior of send() on potentially unconnected sockets differ between platforms:
+------------+-----------+-------------+-----------+
| Event | POSIX | Linux | Windows |
+------------+-----------+-------------+-----------+
| Spurious | ENOTCONN | EWOULDBLOCK | ENOTCONN |
| Failed | ENOTCONN | (cause) | ENOTCONN |
| Successful | (success) | (success) | (success) |
+------------+-----------+-------------+-----------+
*/
if (send(c->socket, NULL, 0, 0) != 0) {
if (sockwouldblock(sockerrno))
return;
int socket_error;
if (!socknotconn(sockerrno))
socket_error = sockerrno;
else {
int len = sizeof socket_error;
getsockopt(c->socket, SOL_SOCKET, SO_ERROR, (void *)&socket_error, &len);
}
if (socket_error) {
logger(DEBUG_CONNECTIONS, LOG_DEBUG, "Error while connecting to %s (%s): %s", c->name, c->hostname, sockstrerror(socket_error));
terminate_connection(c, false);
}
return;
}
c->status.connecting = false;
int result;
socklen_t len = sizeof result;
getsockopt(c->socket, SOL_SOCKET, SO_ERROR, (void *)&result, &len);
if(!result)
finish_connecting(c);
else {
logger(DEBUG_CONNECTIONS, LOG_DEBUG, "Error while connecting to %s (%s): %s", c->name, c->hostname, sockstrerror(result));
terminate_connection(c, false);
return;
}
finish_connecting(c);
}
if(flags & IO_WRITE)

6
src/utils.h
View file

@ -37,8 +37,7 @@ extern const char *winerror(int);
#define sockmsgsize(x) ((x) == WSAEMSGSIZE)
#define sockinprogress(x) ((x) == WSAEINPROGRESS || (x) == WSAEWOULDBLOCK)
#define sockinuse(x) ((x) == WSAEADDRINUSE)
#define sockalready(x) ((x) == WSAEALREADY || (x) == WSAEINVAL || (x) == WSAEWOULDBLOCK) /* See MSDN for connect() */
#define sockisconn(x) ((x) == WSAEISCONN)
#define socknotconn(x) ((x) == WSAENOTCONN)
#else
#define sockerrno errno
#define sockstrerror(x) strerror(x)
@ -46,8 +45,7 @@ extern const char *winerror(int);
#define sockmsgsize(x) ((x) == EMSGSIZE)
#define sockinprogress(x) ((x) == EINPROGRESS)
#define sockinuse(x) ((x) == EADDRINUSE)
#define sockalready(x) ((x) == EALREADY)
#define sockisconn(x) ((x) == EISCONN)
#define socknotconn(x) ((x) == ENOTCONN)
#endif
extern unsigned int bitfield_to_int(const void *bitfield, size_t size);