Index: ChangeLog ================================================================== --- ChangeLog +++ ChangeLog @@ -1,5 +1,13 @@ +2011-05-27 Reinhard Max + + * unix/tclUnixSock.c: Fix [socket -async], so that all addresses + returned by getaddrinfo() are tried, not just the first one. This + requires the event loop to be running while the async connection + is in progress. ***POTENTIAL INCOMPATIBILITY*** + * tests/socket.test: Add a test for the above. + 2011-05-25 Don Porter * library/msgcat/msgcat.tcl: Bump to msgcat 1.4.4. * library/msgcat/pkgIndex.tcl: * unix/Makefile.in Index: tests/socket.test ================================================================== --- tests/socket.test +++ tests/socket.test @@ -68,10 +68,27 @@ testConstraint exec [llength [info commands exec]] # Produce a random port number in the Dynamic/Private range # from 49152 through 65535. proc randport {} { expr {int(rand()*16383+49152)} } + +# Test the latency of tcp connections over the loopback interface. Some OSes +# (e.g. NetBSD) seem to use the Nagle algorithm and delayed ACKs, so it takes +# up to 200ms for a packet sent to localhost to arrive. We're measuring this +# here, so that OSes that don't have this problem can +set server [socket -server {apply {{s a p} {set ::s1 $s}}} 0] +set s2 [socket localhost [lindex [fconfigure $server -sockname] 2]] +vwait s1; close $server +fconfigure $s1 -buffering line +fconfigure $s2 -buffering line +set t1 [clock milliseconds] +puts $s2 test1; gets $s1 +puts $s2 test2; gets $s1 +close $s1; close $s2 +set t2 [clock milliseconds] +set latency [expr {$t2-$t1}] +unset t1 t2 s1 s2 server # If remoteServerIP or remoteServerPort are not set, check in the environment # variables for externally set values. # @@ -582,11 +599,11 @@ set result a:[gets $sock] lappend result b:[gets $sock] fconfigure $sock -blocking 1 puts $s2 two flush $s2 - after idle {set x 1} + after $latency {set x 1}; # NetBSD fails here if we do [after idle] vwait x fconfigure $sock -blocking 0 lappend result c:[gets $sock] } -cleanup { fconfigure $sock -blocking 1 @@ -1697,13 +1714,76 @@ catch {sendCommand exit} } catch {close $commandSocket} catch {close $remoteProcChan} } +unset ::tcl::unsupported::socketAF +test socket-14.0 {[socket -async] when server only listens on one address family} \ + -constraints [list socket supported_any] \ + -setup { + proc accept {s a p} { + global x + puts $s bye + close $s + set x ok + } + set server [socket -server accept -myaddr 127.0.0.1 0] + set port [lindex [fconfigure $server -sockname] 2] + } -body { + set client [socket -async localhost $port] + set after [after 1000 {set x [fconfigure $client -error]}] + vwait x + set x + } -cleanup { + after cancel $after + close $server + close $client + unset x + } -result ok +test socket-14.1 {[socket -async] fileevent while still connecting} \ + -constraints [list socket supported_any] \ + -setup { + proc accept {s a p} { + global x + puts $s bye + close $s + lappend x ok + } + set server [socket -server accept -myaddr 127.0.0.1 0] + set port [lindex [fconfigure $server -sockname] 2] + } -body { + set client [socket -async localhost $port] + fileevent $client writable { + lappend x [expr {[fconfigure $client -error] eq ""}] + } + set after [after 1000 {set x timeout}] + vwait x + vwait x + set x + } -cleanup { + after cancel $after + close $server + close $client + unset x + } -result {ok 1} +test socket-14.2 {[socket -async] fileevent connection refused} \ + -constraints [list socket supported_any] \ + -body { + set client [socket -async localhost 0] + fileevent $client writable {set x [fconfigure $client -error]} + set after [after 1000 {set x timeout}] + vwait x + set x + } -cleanup { + after cancel $after + close $client + unset x + } -result "connection refused" + ::tcltest::cleanupTests flush stdout return # Local Variables: # mode: tcl # fill-column: 78 # End: Index: unix/tclUnixSock.c ================================================================== --- unix/tclUnixSock.c +++ unix/tclUnixSock.c @@ -47,13 +47,31 @@ struct TcpState { Tcl_Channel channel; /* Channel associated with this file. */ TcpFdList *fds; /* The file descriptors of the sockets. */ int flags; /* ORed combination of the bitfields defined * below. */ - Tcl_TcpAcceptProc *acceptProc; + union { + struct { + /* Only needed for server sockets */ + Tcl_TcpAcceptProc *acceptProc; /* Proc to call on accept. */ - ClientData acceptProcData; /* The data for the accept proc. */ + ClientData acceptProcData; + /* The data for the accept proc. */ + }; + struct { + /* + * Only needed for client sockets + */ + struct addrinfo *addrlist; /* addresses to connect to */ + struct addrinfo *addr; /* iterator over addrlist */ + struct addrinfo *myaddrlist; /* local address */ + struct addrinfo *myaddr; /* iterator over myaddrlist */ + int filehandlers; /* Caches FileHandlers that get set up while + * an async socket is not yet connected */ + int status; /* Cache status of async socket */ + }; + }; }; /* * These bits may be ORed together into the "flags" field of a TcpState * structure. @@ -87,13 +105,12 @@ /* * Static routines for this file: */ -static TcpState * CreateClientSocket(Tcl_Interp *interp, int port, - const char *host, const char *myaddr, - int myport, int async); +static int CreateClientSocket(Tcl_Interp *interp, + TcpState *state); static void TcpAccept(ClientData data, int mask); static int TcpBlockModeProc(ClientData data, int mode); static int TcpCloseProc(ClientData instanceData, Tcl_Interp *interp); static int TcpClose2Proc(ClientData instanceData, @@ -630,15 +647,20 @@ if ((len > 1) && (optionName[1] == 'e') && (strncmp(optionName, "-error", len) == 0)) { socklen_t optlen = sizeof(int); int err, ret; - ret = getsockopt(statePtr->fds->fd, SOL_SOCKET, SO_ERROR, - (char *)&err, &optlen); - if (ret < 0) { - err = errno; - } + if (statePtr->status == 0) { + ret = getsockopt(statePtr->fds->fd, SOL_SOCKET, SO_ERROR, + (char *)&err, &optlen); + if (ret < 0) { + err = errno; + } + } else { + err = statePtr->status; + statePtr->status = 0; + } if (err != 0) { Tcl_DStringAppend(dsPtr, Tcl_ErrnoMsg(err), -1); } return TCL_OK; } @@ -783,20 +805,21 @@ int mask) /* Events of interest; an OR-ed combination of * TCL_READABLE, TCL_WRITABLE and * TCL_EXCEPTION. */ { TcpState *statePtr = (TcpState *) instanceData; - TcpFdList *fds; - - for (fds = statePtr->fds; fds != NULL; fds = fds->next) { - if (mask) { - Tcl_CreateFileHandler(fds->fd, mask, - (Tcl_FileProc *) Tcl_NotifyChannel, - (ClientData) statePtr->channel); - } else { - Tcl_DeleteFileHandler(fds->fd); - } + + if (statePtr->flags & TCP_ASYNC_CONNECT) { + /* Async sockets use a FileHandler internally while connecting, so we + * need to cache this request until the connection has succeeded. */ + statePtr->filehandlers = mask; + } else if (mask) { + Tcl_CreateFileHandler(statePtr->fds->fd, mask, + (Tcl_FileProc *) Tcl_NotifyChannel, + (ClientData) statePtr->channel); + } else { + Tcl_DeleteFileHandler(statePtr->fds->fd); } } /* *---------------------------------------------------------------------- @@ -826,175 +849,174 @@ TcpState *statePtr = (TcpState *) instanceData; *handlePtr = INT2PTR(statePtr->fds->fd); return TCL_OK; } + +static void +TcpAsyncCallback( + ClientData clientData, /* The socket state. */ + int mask) /* Events of interest; an OR-ed combination of + * TCL_READABLE, TCL_WRITABLE and + * TCL_EXCEPTION. */ +{ + CreateClientSocket(NULL, clientData); +} /* *---------------------------------------------------------------------- * - * CreateSocket -- + * CreateClientSocket -- * - * This function opens a new socket in client or server mode and - * initializes the TcpState structure. + * This function opens a new socket in client mode. * * Results: - * Returns a new TcpState, or NULL with an error in the interp's result, - * if interp is not NULL. + * TCL_OK, if the socket was successfully connected or an asynchronous + * connection is in progress. If an error occurs, TCL_ERROR is returned + * and an error message is left in interp. * * Side effects: * Opens a socket. * *---------------------------------------------------------------------- */ -static TcpState * +static int CreateClientSocket( Tcl_Interp *interp, /* For error reporting; can be NULL. */ - int port, /* Port number to open. */ - const char *host, /* Name of host on which to open port. */ - const char *myaddr, /* Optional client-side address. - * NULL implies INADDR_ANY/in6addr_any */ - int myport, /* Optional client-side port */ - int async) /* If nonzero and creating a client socket, - * attempt to do an async connect. Otherwise - * do a synchronous connect or bind. */ -{ - int status = -1, connected = 0, sock = -1; - struct addrinfo *addrlist = NULL, *addrPtr; - /* Socket address */ - struct addrinfo *myaddrlist = NULL, *myaddrPtr; - /* Socket address for client */ - TcpState *statePtr; - const char *errorMsg = NULL; - - if (!TclCreateSocketAddress(interp, &addrlist, host, port, 0, &errorMsg)) { - goto error; - } - if (!TclCreateSocketAddress(interp, &myaddrlist, myaddr, myport, 1, &errorMsg)) { - goto error; - } - - for (addrPtr = addrlist; addrPtr != NULL; - addrPtr = addrPtr->ai_next) { - for (myaddrPtr = myaddrlist; myaddrPtr != NULL; - myaddrPtr = myaddrPtr->ai_next) { + TcpState *state) +{ + socklen_t optlen; + int in_coro = (state->addr != NULL); + int status, connected = 0; + int async = state->flags & TCP_ASYNC_CONNECT; + + if (in_coro) { + goto coro_continue; + } + + for (state->addr = state->addrlist; state->addr != NULL; + state->addr = state->addr->ai_next) { + + status = -1; + + for (state->myaddr = state->myaddrlist; state->myaddr != NULL; + state->myaddr = state->myaddr->ai_next) { int reuseaddr; /* * No need to try combinations of local and remote addresses of * different families. */ - if (myaddrPtr->ai_family != addrPtr->ai_family) { + if (state->myaddr->ai_family != state->addr->ai_family) { continue; } - sock = socket(addrPtr->ai_family, SOCK_STREAM, 0); - if (sock < 0) { + /* + * Close the socket if it is still open from the last unsuccessful + * iteration. + */ + if (state->fds->fd >= 0) { + close(state->fds->fd); + state->fds->fd = -1; + } + + state->fds->fd = socket(state->addr->ai_family, SOCK_STREAM, 0); + if (state->fds->fd < 0) { continue; } /* * Set the close-on-exec flag so that the socket will not get * inherited by child processes. */ - fcntl(sock, F_SETFD, FD_CLOEXEC); + fcntl(state->fds->fd, F_SETFD, FD_CLOEXEC); /* * Set kernel space buffering */ - TclSockMinimumBuffers(INT2PTR(sock), SOCKET_BUFSIZE); + TclSockMinimumBuffers(INT2PTR(state->fds->fd), SOCKET_BUFSIZE); if (async) { - status = TclUnixSetBlockingMode(sock, TCL_MODE_NONBLOCKING); + status = TclUnixSetBlockingMode(state->fds->fd, TCL_MODE_NONBLOCKING); if (status < 0) { - goto looperror; + continue; } } reuseaddr = 1; - (void) setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, + (void) setsockopt(state->fds->fd, SOL_SOCKET, SO_REUSEADDR, (char *) &reuseaddr, sizeof(reuseaddr)); - status = bind(sock, myaddrPtr->ai_addr, myaddrPtr->ai_addrlen); + status = bind(state->fds->fd, state->myaddr->ai_addr, + state->myaddr->ai_addrlen); if (status < 0) { - goto looperror; + continue; } /* * Attempt to connect. The connect may fail at present with an * EINPROGRESS but at a later time it will complete. The caller * will set up a file handler on the socket if she is interested * in being informed when the connect completes. */ - status = connect(sock, addrPtr->ai_addr, addrPtr->ai_addrlen); - if (status < 0 && errno == EINPROGRESS) { - status = 0; - } - if (status == 0) { - connected = 1; - break; - } - looperror: - if (sock != -1) { - close(sock); - sock = -1; - } - } - if (connected) { - break; - } - status = -1; - if (sock >= 0) { - close(sock); - sock = -1; - } - } - if (async) { - /* - * Restore blocking mode. - */ - - status = TclUnixSetBlockingMode(sock, TCL_MODE_BLOCKING); - } - -error: - if (addrlist) { - freeaddrinfo(addrlist); - } - if (myaddrlist) { - freeaddrinfo(myaddrlist); - } - - if (status < 0) { - if (interp != NULL) { - Tcl_AppendResult(interp, "couldn't open socket: ", - Tcl_PosixError(interp), NULL); - if (errorMsg != NULL) { - Tcl_AppendResult(interp, " (", errorMsg, ")", NULL); - } - } - if (sock != -1) { - close(sock); - } - return NULL; - } - - /* - * Allocate a new TcpState for this socket. - */ - - statePtr = ckalloc(sizeof(TcpState)); - statePtr->flags = async ? TCP_ASYNC_CONNECT : 0; - statePtr->fds = ckalloc(sizeof(TcpFdList)); - memset(statePtr->fds, (int) 0, sizeof(TcpFdList)); - statePtr->fds->fd = sock; - - return statePtr; + status = connect(state->fds->fd, state->addr->ai_addr, + state->addr->ai_addrlen); + if (status < 0 && errno == EINPROGRESS) { + Tcl_CreateFileHandler(state->fds->fd, TCL_WRITABLE, + TcpAsyncCallback, state); + return TCL_OK; + + coro_continue: + Tcl_DeleteFileHandler(state->fds->fd); + /* + * Read the error state from the socket, to see if the async + * connection has succeeded or failed and store the status in + * the socket state for later retrieval by [fconfigure -error] + */ + optlen = sizeof(int); + getsockopt(state->fds->fd, SOL_SOCKET, SO_ERROR, + (char *)&status, &optlen); + state->status = status; + } + if (status == 0) { + goto out; + } + } + } + +out: + + freeaddrinfo(state->addrlist); + freeaddrinfo(state->myaddrlist); + + if (async) { + CLEAR_BITS(state->flags, TCP_ASYNC_CONNECT); + TcpWatchProc(state, state->filehandlers); + TclUnixSetBlockingMode(state->fds->fd, TCL_MODE_BLOCKING); + } + + if (status < 0) { + if (in_coro) { + Tcl_NotifyChannel(state->fds->fd, TCL_WRITABLE); + } else { + if (interp != NULL) { + Tcl_AppendResult(interp, "couldn't open socket: ", + Tcl_PosixError(interp), NULL); + } + if (state->fds->fd != -1) { + close(state->fds->fd); + } + ckfree(state->fds); + ckfree(state); + return TCL_ERROR; + } + } + return TCL_OK; } /* *---------------------------------------------------------------------- * @@ -1021,35 +1043,67 @@ int myport, /* Client-side port */ int async) /* If nonzero, attempt to do an asynchronous * connect. Otherwise we do a blocking * connect. */ { - TcpState *statePtr; - char channelName[16 + TCL_INTEGER_SPACE]; + TcpState *state; + const char *errorMsg = NULL; + struct addrinfo *addrlist, *myaddrlist; + char channelName[4+16+1]; /* "sock" + up to 16 hex chars + \0 */ + + + /* + * Do the name lookups for the local and remote addresses. + */ + if (!TclCreateSocketAddress(interp, &addrlist, host, port, 0, &errorMsg)) { + goto error; + } + if (!TclCreateSocketAddress(interp, &myaddrlist, myaddr, myport, 1, + &errorMsg)) { + freeaddrinfo(addrlist); + goto error; + } + + /* + * Allocate a new TcpState for this socket. + */ + state = ckalloc(sizeof(TcpState)); + memset(state, 0, sizeof(TcpState)); + state->flags = async ? TCP_ASYNC_CONNECT : 0; + state->addrlist = addrlist; + state->myaddrlist = myaddrlist; + state->fds = ckalloc(sizeof(TcpFdList)); + memset(state->fds, (int) 0, sizeof(TcpFdList)); + state->fds->fd = -1; /* * Create a new client socket and wrap it in a channel. */ - - statePtr = CreateClientSocket(interp, port, host, myaddr, myport, async); - if (statePtr == NULL) { - return NULL; - } - - statePtr->acceptProc = NULL; - statePtr->acceptProcData = NULL; - - sprintf(channelName, "sock%d", statePtr->fds->fd); - - statePtr->channel = Tcl_CreateChannel(&tcpChannelType, channelName, - statePtr, (TCL_READABLE | TCL_WRITABLE)); - if (Tcl_SetChannelOption(interp, statePtr->channel, "-translation", - "auto crlf") == TCL_ERROR) { - Tcl_Close(NULL, statePtr->channel); - return NULL; - } - return statePtr->channel; + if (CreateClientSocket(interp, state) != TCL_OK) { + goto error; + } + + sprintf(channelName, "sock%lx", (long)state); + + state->channel = Tcl_CreateChannel(&tcpChannelType, channelName, + state, (TCL_READABLE | TCL_WRITABLE)); + if (Tcl_SetChannelOption(interp, state->channel, "-translation", + "auto crlf") == TCL_ERROR) { + Tcl_Close(NULL, state->channel); + return NULL; + } + return state->channel; + +error: + if (interp != NULL) { + Tcl_AppendResult(interp, "couldn't open socket: ", + Tcl_PosixError(interp), NULL); + if (errorMsg != NULL) { + Tcl_AppendResult(interp, " (", errorMsg, ")", NULL); + } + } + return NULL; } /* *---------------------------------------------------------------------- * @@ -1098,16 +1152,15 @@ { TcpState *statePtr; char channelName[16 + TCL_INTEGER_SPACE]; statePtr = ckalloc(sizeof(TcpState)); + memset(statePtr, 0, sizeof(TcpState)); statePtr->fds = ckalloc(sizeof(TcpFdList)); memset(statePtr->fds, (int) 0, sizeof(TcpFdList)); statePtr->fds->fd = PTR2INT(sock); statePtr->flags = 0; - statePtr->acceptProc = NULL; - statePtr->acceptProcData = NULL; sprintf(channelName, "sock%d", statePtr->fds->fd); statePtr->channel = Tcl_CreateChannel(&tcpChannelType, channelName, statePtr, mode); @@ -1236,10 +1289,11 @@ /* * Allocate a new TcpState for this socket. */ statePtr = ckalloc(sizeof(TcpState)); + memset(statePtr, 0, sizeof(TcpState)); statePtr->fds = newfds; statePtr->acceptProc = acceptProc; statePtr->acceptProcData = acceptProcData; sprintf(channelName, "sock%d", sock); } else { @@ -1321,17 +1375,15 @@ */ (void) fcntl(newsock, F_SETFD, FD_CLOEXEC); newSockState = ckalloc(sizeof(TcpState)); - + memset(newSockState, 0, sizeof(TcpState)); newSockState->flags = 0; newSockState->fds = ckalloc(sizeof(TcpFdList)); memset(newSockState->fds, (int) 0, sizeof(TcpFdList)); newSockState->fds->fd = newsock; - newSockState->acceptProc = NULL; - newSockState->acceptProcData = NULL; sprintf(channelName, "sock%d", newsock); newSockState->channel = Tcl_CreateChannel(&tcpChannelType, channelName, newSockState, (TCL_READABLE | TCL_WRITABLE));