From 19755ae8971d97cfc092add10ceed6ab40f011bd Mon Sep 17 00:00:00 2001 From: max Date: Fri, 27 May 2011 18:36:26 +0000 Subject: Fix [socket -async] for DNS names with more than one address --- ChangeLog | 8 ++ tests/socket.test | 22 +++++ unix/tclUnixSock.c | 231 ++++++++++++++++++++++++++++++----------------------- 3 files changed, 163 insertions(+), 98 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5c4c72a..f7b11cc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +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. diff --git a/tests/socket.test b/tests/socket.test index 83bad09..1bb9b79 100644 --- a/tests/socket.test +++ b/tests/socket.test @@ -1699,6 +1699,28 @@ if {$remoteProcChan ne ""} { catch {close $commandSocket} catch {close $remoteProcChan} } +unset ::tcl::unsupported::socketAF +test socket-14.0 {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] + # fileevent $client readable [list set x [fconfigure $client -error]] + after 1000 {set x [fconfigure $client -error]} + vwait x + set x + } -cleanup { + close $server + close $client + } -result ok ::tcltest::cleanupTests flush stdout return diff --git a/unix/tclUnixSock.c b/unix/tclUnixSock.c index cb72759..f6a1bf2 100644 --- a/unix/tclUnixSock.c +++ b/unix/tclUnixSock.c @@ -49,9 +49,22 @@ struct TcpState { 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; + struct addrinfo *myaddrlist; + struct addrinfo *addr; + struct addrinfo *myaddr; + }; + }; }; /* @@ -89,9 +102,8 @@ struct TcpState { * 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, @@ -829,17 +841,27 @@ TcpGetHandleProc( 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. @@ -847,37 +869,22 @@ TcpGetHandleProc( *---------------------------------------------------------------------- */ -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. */ + TcpState *state) { - 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; + int status = -1, connected = 0; + int async = state->flags & TCP_ASYNC_CONNECT; - if (!TclCreateSocketAddress(interp, &addrlist, host, port, 0, &errorMsg)) { - goto error; + if (state->addr != NULL) { + goto coro_continue; } - 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) { + + for (state->addr = state->addrlist; state->addr != NULL; + state->addr = state->addr->ai_next) { + for (state->myaddr = state->myaddrlist; state->myaddr != NULL; + state->myaddr = state->myaddr->ai_next) { int reuseaddr; /* @@ -885,12 +892,12 @@ CreateClientSocket( * 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) { + state->fds->fd = socket(state->addr->ai_family, SOCK_STREAM, 0); + if (state->fds->fd < 0) { continue; } @@ -899,25 +906,26 @@ CreateClientSocket( * 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; } } 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; } @@ -929,27 +937,39 @@ CreateClientSocket( * in being informed when the connect completes. */ - status = connect(sock, addrPtr->ai_addr, addrPtr->ai_addrlen); + status = connect(state->fds->fd, state->addr->ai_addr, + state->addr->ai_addrlen); if (status < 0 && errno == EINPROGRESS) { - status = 0; - } + Tcl_CreateFileHandler(state->fds->fd, TCL_WRITABLE, + TcpAsyncCallback, state); + // fprintf(stderr, "here: %d \n", state->fds->fd); + return TCL_OK; + coro_continue: + do { + socklen_t optlen = sizeof(int); + Tcl_DeleteFileHandler(state->fds->fd); + getsockopt(state->fds->fd, SOL_SOCKET, SO_ERROR, + (char *)&status, &optlen); + // fprintf(stderr, "there: %d \n", state->fds->fd); + } while (0); + } if (status == 0) { connected = 1; break; } looperror: - if (sock != -1) { - close(sock); - sock = -1; + if (state->fds->fd != -1) { + close(state->fds->fd); + state->fds->fd = -1; } } if (connected) { break; } status = -1; - if (sock >= 0) { - close(sock); - sock = -1; + if (state->fds->fd >= 0) { + close(state->fds->fd); + state->fds->fd = -1; } } if (async) { @@ -957,42 +977,25 @@ CreateClientSocket( * Restore blocking mode. */ - status = TclUnixSetBlockingMode(sock, TCL_MODE_BLOCKING); + status = TclUnixSetBlockingMode(state->fds->fd, TCL_MODE_BLOCKING); } -error: - if (addrlist) { - freeaddrinfo(addrlist); - } - if (myaddrlist) { - freeaddrinfo(myaddrlist); - } - - if (status < 0) { - if (interp != NULL) { + freeaddrinfo(state->addrlist); + freeaddrinfo(state->myaddrlist); + + if (status < 0 && !async) { + if (interp != NULL) { Tcl_AppendResult(interp, "couldn't open socket: ", - Tcl_PosixError(interp), NULL); - if (errorMsg != NULL) { - Tcl_AppendResult(interp, " (", errorMsg, ")", NULL); - } + Tcl_PosixError(interp), NULL); } - if (sock != -1) { - close(sock); + if (state->fds->fd != -1) { + close(state->fds->fd); } - return NULL; + ckfree(state->fds); + ckfree(state); + return TCL_ERROR; } - - /* - * 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; + return TCL_OK; } /* @@ -1023,31 +1026,63 @@ Tcl_OpenTcpClient( * 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 */ + /* - * Create a new client socket and wrap it in a channel. + * Do the name lookups for the local and remote addresses. */ - - statePtr = CreateClientSocket(interp, port, host, myaddr, myport, async); - if (statePtr == NULL) { - return NULL; + if (!TclCreateSocketAddress(interp, &addrlist, host, port, 0, &errorMsg)) { + goto error; + } + if (!TclCreateSocketAddress(interp, &myaddrlist, myaddr, myport, 1, + &errorMsg)) { + freeaddrinfo(addrlist); + goto error; } - statePtr->acceptProc = NULL; - statePtr->acceptProcData = NULL; + /* + * 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; - sprintf(channelName, "sock%d", statePtr->fds->fd); + /* + * Create a new client socket and wrap it in a channel. + */ + if (CreateClientSocket(interp, state) != TCL_OK) { + goto error; + } - statePtr->channel = Tcl_CreateChannel(&tcpChannelType, channelName, - statePtr, (TCL_READABLE | TCL_WRITABLE)); - if (Tcl_SetChannelOption(interp, statePtr->channel, "-translation", + 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, statePtr->channel); + Tcl_Close(NULL, state->channel); return NULL; } - return statePtr->channel; + 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; } /* -- cgit v0.12 From 2f67cb7c57ed82cb80c4e9a3905850869b9c63c4 Mon Sep 17 00:00:00 2001 From: max Date: Mon, 30 May 2011 18:04:42 +0000 Subject: * Fix setting up of [fileevent] while an async socket is still in progress * Cache async socket errors for later use by [fconfigure -error] * Add tests for the above --- tests/socket.test | 39 +++++++++++++++++++-- unix/tclUnixSock.c | 101 +++++++++++++++++++++++++++++++---------------------- 2 files changed, 96 insertions(+), 44 deletions(-) diff --git a/tests/socket.test b/tests/socket.test index 1bb9b79..dd57a3d 100644 --- a/tests/socket.test +++ b/tests/socket.test @@ -1700,7 +1700,7 @@ catch {close $commandSocket} catch {close $remoteProcChan} } unset ::tcl::unsupported::socketAF -test socket-14.0 {async when server only listens on one address family} \ +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} { @@ -1713,7 +1713,6 @@ test socket-14.0 {async when server only listens on one address family} \ set port [lindex [fconfigure $server -sockname] 2] } -body { set client [socket -async localhost $port] - # fileevent $client readable [list set x [fconfigure $client -error]] after 1000 {set x [fconfigure $client -error]} vwait x set x @@ -1721,6 +1720,42 @@ test socket-14.0 {async when server only listens on one address family} \ close $server close $client } -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 + set x ok + } + set server [socket -server accept -myaddr 127.0.0.1 2222] + set port [lindex [fconfigure $server -sockname] 2] + } -body { + set client [socket -async localhost $port] + fileevent $client readable {lappend x [fconfigure $client -error]} + set after [after 1000 {set x timeout}] + vwait x + vwait x + set x + } -cleanup { + after cancel $after + close $server + close $client + } -result {ok {}} +test socket-14.2 {[socket -async] fileevent connection refused} \ + -constraints [list socket supported_any] \ + -body { + set client [socket -async localhost 0] + fileevent $client readable {set x [fconfigure $client -error]} + set after [after 1000 {set x timeout}] + vwait x + set x + } -cleanup { + after cancel $after + close $client + } -result "connection refused" + ::tcltest::cleanupTests flush stdout return diff --git a/unix/tclUnixSock.c b/unix/tclUnixSock.c index f6a1bf2..823942a 100644 --- a/unix/tclUnixSock.c +++ b/unix/tclUnixSock.c @@ -58,11 +58,16 @@ struct TcpState { /* The data for the accept proc. */ }; struct { - /* Only needed for client sockets */ - struct addrinfo *addrlist; - struct addrinfo *myaddrlist; - struct addrinfo *addr; - struct addrinfo *myaddr; + /* + * 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 */ }; }; }; @@ -644,11 +649,16 @@ TcpGetOptionProc( 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); } @@ -797,16 +807,17 @@ TcpWatchProc( * 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); } } @@ -874,7 +885,7 @@ CreateClientSocket( Tcl_Interp *interp, /* For error reporting; can be NULL. */ TcpState *state) { - int status = -1, connected = 0; + int status, connected = 0; int async = state->flags & TCP_ASYNC_CONNECT; if (state->addr != NULL) { @@ -883,6 +894,9 @@ CreateClientSocket( 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; @@ -896,6 +910,15 @@ CreateClientSocket( continue; } + /* + * 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; @@ -917,7 +940,7 @@ CreateClientSocket( if (async) { status = TclUnixSetBlockingMode(state->fds->fd, TCL_MODE_NONBLOCKING); if (status < 0) { - goto looperror; + continue; } } @@ -927,7 +950,7 @@ CreateClientSocket( status = bind(state->fds->fd, state->myaddr->ai_addr, state->myaddr->ai_addrlen); if (status < 0) { - goto looperror; + continue; } /* @@ -942,35 +965,24 @@ CreateClientSocket( if (status < 0 && errno == EINPROGRESS) { Tcl_CreateFileHandler(state->fds->fd, TCL_WRITABLE, TcpAsyncCallback, state); - // fprintf(stderr, "here: %d \n", state->fds->fd); return TCL_OK; coro_continue: do { socklen_t optlen = sizeof(int); Tcl_DeleteFileHandler(state->fds->fd); getsockopt(state->fds->fd, SOL_SOCKET, SO_ERROR, - (char *)&status, &optlen); - // fprintf(stderr, "there: %d \n", state->fds->fd); + (char *)&status, &optlen); + state->status = status; } while (0); } if (status == 0) { connected = 1; break; } - looperror: - if (state->fds->fd != -1) { - close(state->fds->fd); - state->fds->fd = -1; - } } if (connected) { break; } - status = -1; - if (state->fds->fd >= 0) { - close(state->fds->fd); - state->fds->fd = -1; - } } if (async) { /* @@ -983,7 +995,14 @@ CreateClientSocket( freeaddrinfo(state->addrlist); freeaddrinfo(state->myaddrlist); - if (status < 0 && !async) { + if (async) { + CLEAR_BITS(state->flags, TCP_ASYNC_CONNECT); + if (state->filehandlers != 0) { + TcpWatchProc(state, state->filehandlers); + } + return TCL_OK; + } + if (status < 0) { if (interp != NULL) { Tcl_AppendResult(interp, "couldn't open socket: ", Tcl_PosixError(interp), NULL); @@ -1135,12 +1154,11 @@ TclpMakeTcpClientChannelMode( 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); @@ -1273,6 +1291,7 @@ Tcl_OpenTcpServer( */ statePtr = ckalloc(sizeof(TcpState)); + memset(statePtr, 0, sizeof(TcpState)); statePtr->fds = newfds; statePtr->acceptProc = acceptProc; statePtr->acceptProcData = acceptProcData; @@ -1358,13 +1377,11 @@ TcpAccept( (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, -- cgit v0.12 From b8ee85e7eb1b906ac79f4fae165c4e8cf0e9faf4 Mon Sep 17 00:00:00 2001 From: max Date: Wed, 1 Jun 2011 15:30:28 +0000 Subject: * Improve socket.test by checking the latency on the loopback address and use that for some of the tests instead of fixed "big enough" times. * Improve correctness of [socket -async] in some error cases. --- tests/socket.test | 37 ++++++++++++++++++++++++------ unix/tclUnixSock.c | 66 +++++++++++++++++++++++++++--------------------------- 2 files changed, 63 insertions(+), 40 deletions(-) diff --git a/tests/socket.test b/tests/socket.test index dd57a3d..b121022 100644 --- a/tests/socket.test +++ b/tests/socket.test @@ -71,6 +71,23 @@ testConstraint exec [llength [info commands exec]] # 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. # @@ -584,7 +601,7 @@ test socket_$af-2.11 {detecting new data} -constraints [list socket supported_$a 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] @@ -1713,12 +1730,14 @@ test socket-14.0 {[socket -async] when server only listens on one address family set port [lindex [fconfigure $server -sockname] 2] } -body { set client [socket -async localhost $port] - after 1000 {set x [fconfigure $client -error]} + 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] \ @@ -1727,13 +1746,15 @@ test socket-14.1 {[socket -async] fileevent while still connecting} \ global x puts $s bye close $s - set x ok + lappend x ok } - set server [socket -server accept -myaddr 127.0.0.1 2222] + 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 readable {lappend x [fconfigure $client -error]} + fileevent $client writable { + lappend x [expr {[fconfigure $client -error] eq ""}] + } set after [after 1000 {set x timeout}] vwait x vwait x @@ -1742,18 +1763,20 @@ test socket-14.1 {[socket -async] fileevent while still connecting} \ after cancel $after close $server close $client - } -result {ok {}} + 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 readable {set x [fconfigure $client -error]} + 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 diff --git a/unix/tclUnixSock.c b/unix/tclUnixSock.c index 823942a..981162d 100644 --- a/unix/tclUnixSock.c +++ b/unix/tclUnixSock.c @@ -885,10 +885,12 @@ CreateClientSocket( Tcl_Interp *interp, /* For error reporting; can be NULL. */ TcpState *state) { + socklen_t optlen; + int in_coro = (state->addr != NULL); int status, connected = 0; int async = state->flags & TCP_ASYNC_CONNECT; - if (state->addr != NULL) { + if (in_coro) { goto coro_continue; } @@ -966,53 +968,51 @@ CreateClientSocket( Tcl_CreateFileHandler(state->fds->fd, TCL_WRITABLE, TcpAsyncCallback, state); return TCL_OK; + coro_continue: - do { - socklen_t optlen = sizeof(int); - Tcl_DeleteFileHandler(state->fds->fd); - getsockopt(state->fds->fd, SOL_SOCKET, SO_ERROR, - (char *)&status, &optlen); - state->status = status; - } while (0); + 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) { - connected = 1; - break; + goto out; } } - if (connected) { - break; - } } - if (async) { - /* - * Restore blocking mode. - */ - status = TclUnixSetBlockingMode(state->fds->fd, TCL_MODE_BLOCKING); - } +out: freeaddrinfo(state->addrlist); freeaddrinfo(state->myaddrlist); if (async) { CLEAR_BITS(state->flags, TCP_ASYNC_CONNECT); - if (state->filehandlers != 0) { - TcpWatchProc(state, state->filehandlers); - } - return TCL_OK; + TcpWatchProc(state, state->filehandlers); + TclUnixSetBlockingMode(state->fds->fd, TCL_MODE_BLOCKING); } + if (status < 0) { - 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; + 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; } -- cgit v0.12 From a7f1ab1afd109c2c02de573a66aaab15bfbdeab1 Mon Sep 17 00:00:00 2001 From: max Date: Mon, 6 Jun 2011 15:07:23 +0000 Subject: * Don't use port 0 for test 14.2 as it fails in different ways on Linux and NetBSD. * Unify channel name creation. * Prevent error messages from appearing twice. * Double the measured latency in socket.test to be on the safe side. --- tests/socket.test | 4 ++-- unix/tclUnixSock.c | 59 +++++++++++++++++++++++++++--------------------------- 2 files changed, 31 insertions(+), 32 deletions(-) diff --git a/tests/socket.test b/tests/socket.test index b121022..39dc8de 100644 --- a/tests/socket.test +++ b/tests/socket.test @@ -85,7 +85,7 @@ puts $s2 test1; gets $s1 puts $s2 test2; gets $s1 close $s1; close $s2 set t2 [clock milliseconds] -set latency [expr {$t2-$t1}] +set latency [expr {($t2-$t1)*2}]; # doubled as a safety margin unset t1 t2 s1 s2 server # If remoteServerIP or remoteServerPort are not set, check in the environment @@ -1768,7 +1768,7 @@ test socket-14.1 {[socket -async] fileevent while still connecting} \ test socket-14.2 {[socket -async] fileevent connection refused} \ -constraints [list socket supported_any] \ -body { - set client [socket -async localhost 0] + set client [socket -async localhost [randport]] fileevent $client writable {set x [fconfigure $client -error]} set after [after 1000 {set x timeout}] vwait x diff --git a/unix/tclUnixSock.c b/unix/tclUnixSock.c index 981162d..0d6b1d0 100644 --- a/unix/tclUnixSock.c +++ b/unix/tclUnixSock.c @@ -20,6 +20,10 @@ #define SET_BITS(var, bits) ((var) |= (bits)) #define CLEAR_BITS(var, bits) ((var) &= ~(bits)) +/* "sock" + a pointer in hex + \0 */ +#define SOCK_CHAN_LENGTH 4 + sizeof(void*) * 2 + 1 +#define SOCK_TEMPLATE "sock%lx" + /* * This is needed to comply with the strict aliasing rules of GCC, but it also * simplifies casting between the different sockaddr types. @@ -887,7 +891,7 @@ CreateClientSocket( { socklen_t optlen; int in_coro = (state->addr != NULL); - int status, connected = 0; + int status; int async = state->flags & TCP_ASYNC_CONNECT; if (in_coro) { @@ -1000,7 +1004,7 @@ out: if (status < 0) { if (in_coro) { - Tcl_NotifyChannel(state->fds->fd, TCL_WRITABLE); + Tcl_NotifyChannel(state->channel, TCL_WRITABLE); } else { if (interp != NULL) { Tcl_AppendResult(interp, "couldn't open socket: ", @@ -1047,20 +1051,25 @@ Tcl_OpenTcpClient( { TcpState *state; const char *errorMsg = NULL; - struct addrinfo *addrlist, *myaddrlist; - char channelName[4+16+1]; /* "sock" + up to 16 hex chars + \0 */ - + struct addrinfo *addrlist = NULL, *myaddrlist = NULL; + char channelName[SOCK_CHAN_LENGTH]; /* * 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; + if (!TclCreateSocketAddress(interp, &addrlist, host, port, 0, &errorMsg) || + !TclCreateSocketAddress(interp, &myaddrlist, myaddr, myport, 1, &errorMsg)) { + if (addrlist != NULL) { + freeaddrinfo(addrlist); + } + if (interp != NULL) { + Tcl_AppendResult(interp, "couldn't open socket: ", + Tcl_PosixError(interp), NULL); + if (errorMsg != NULL) { + Tcl_AppendResult(interp, " (", errorMsg, ")", NULL); + } + } + return NULL; } /* @@ -1079,10 +1088,10 @@ Tcl_OpenTcpClient( * Create a new client socket and wrap it in a channel. */ if (CreateClientSocket(interp, state) != TCL_OK) { - goto error; + return NULL; } - sprintf(channelName, "sock%lx", (long)state); + sprintf(channelName, SOCK_TEMPLATE, (long)state); state->channel = Tcl_CreateChannel(&tcpChannelType, channelName, state, (TCL_READABLE | TCL_WRITABLE)); @@ -1092,16 +1101,6 @@ Tcl_OpenTcpClient( 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; } /* @@ -1151,7 +1150,7 @@ TclpMakeTcpClientChannelMode( * TCL_WRITABLE to indicate file mode. */ { TcpState *statePtr; - char channelName[16 + TCL_INTEGER_SPACE]; + char channelName[SOCK_CHAN_LENGTH]; statePtr = ckalloc(sizeof(TcpState)); memset(statePtr, 0, sizeof(TcpState)); @@ -1160,7 +1159,7 @@ TclpMakeTcpClientChannelMode( statePtr->fds->fd = PTR2INT(sock); statePtr->flags = 0; - sprintf(channelName, "sock%d", statePtr->fds->fd); + sprintf(channelName, SOCK_TEMPLATE, (long)statePtr); statePtr->channel = Tcl_CreateChannel(&tcpChannelType, channelName, statePtr, mode); @@ -1202,7 +1201,7 @@ Tcl_OpenTcpServer( int status = 0, sock = -1, reuseaddr = 1, chosenport = 0; struct addrinfo *addrlist = NULL, *addrPtr; /* socket address */ TcpState *statePtr = NULL; - char channelName[16 + TCL_INTEGER_SPACE]; + char channelName[SOCK_CHAN_LENGTH]; const char *errorMsg = NULL; TcpFdList *fds = NULL, *newfds; @@ -1295,7 +1294,7 @@ Tcl_OpenTcpServer( statePtr->fds = newfds; statePtr->acceptProc = acceptProc; statePtr->acceptProcData = acceptProcData; - sprintf(channelName, "sock%d", sock); + sprintf(channelName, SOCK_TEMPLATE, (long)statePtr); } else { fds->next = newfds; } @@ -1360,7 +1359,7 @@ TcpAccept( TcpState *newSockState; /* State for new socket. */ address addr; /* The remote address */ socklen_t len; /* For accept interface */ - char channelName[16 + TCL_INTEGER_SPACE]; + char channelName[SOCK_CHAN_LENGTH]; char host[NI_MAXHOST], port[NI_MAXSERV]; len = sizeof(addr); @@ -1383,7 +1382,7 @@ TcpAccept( memset(newSockState->fds, (int) 0, sizeof(TcpFdList)); newSockState->fds->fd = newsock; - sprintf(channelName, "sock%d", newsock); + sprintf(channelName, SOCK_TEMPLATE, (long)newSockState); newSockState->channel = Tcl_CreateChannel(&tcpChannelType, channelName, newSockState, (TCL_READABLE | TCL_WRITABLE)); -- cgit v0.12 From cd37602cf4924672b4219fdf144d76e2cf773947 Mon Sep 17 00:00:00 2001 From: max Date: Tue, 7 Jun 2011 12:53:13 +0000 Subject: Fix bug#3164655: getaddrinfo() crash on HP-UX --- generic/tclIOSock.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/generic/tclIOSock.c b/generic/tclIOSock.c index ab2b094..aabd67d 100644 --- a/generic/tclIOSock.c +++ b/generic/tclIOSock.c @@ -178,8 +178,11 @@ TclCreateSocketAddress( } hints.ai_socktype = SOCK_STREAM; -#if defined(AI_ADDRCONFIG) && !defined(_AIX) - /* Missing on: OpenBSD, NetBSD. Causes failure when used on AIX 5.1 */ +#if defined(AI_ADDRCONFIG) && !defined(_AIX) && !defined(__hpux) + /* + * Missing on: OpenBSD, NetBSD. + * Causes failure when used on AIX 5.1 and HP-UX + */ hints.ai_flags |= AI_ADDRCONFIG; #endif if (willBind) { -- cgit v0.12 From 938cb914fee8405b397ae0823e16444bc9eb7c0c Mon Sep 17 00:00:00 2001 From: max Date: Tue, 7 Jun 2011 14:31:59 +0000 Subject: Fix bug#3084338, a memleak when a [socket -async] was closed before the connection had succeeded or failed. --- unix/tclUnixSock.c | 51 ++++++++++++++++++++++----------------------------- 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/unix/tclUnixSock.c b/unix/tclUnixSock.c index 0d6b1d0..39fb375 100644 --- a/unix/tclUnixSock.c +++ b/unix/tclUnixSock.c @@ -53,27 +53,21 @@ struct TcpState { TcpFdList *fds; /* The file descriptors of the sockets. */ int flags; /* ORed combination of the bitfields defined * below. */ - union { - struct { - /* Only needed for server sockets */ - Tcl_TcpAcceptProc *acceptProc; - /* Proc to call on accept. */ - 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 */ - }; - }; + /* + * Only needed for server sockets + */ + Tcl_TcpAcceptProc *acceptProc; /* Proc to call on accept. */ + ClientData acceptProcData; /* The data for the accept proc. */ + /* + * 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 */ }; /* @@ -551,6 +545,12 @@ TcpCloseProc( } ckfree(fds); } + if (statePtr->addrlist != NULL) { + freeaddrinfo(statePtr->addrlist); + } + if (statePtr->myaddrlist != NULL) { + freeaddrinfo(statePtr->myaddrlist); + } ckfree(statePtr); return errorCode; } @@ -993,9 +993,6 @@ CreateClientSocket( out: - freeaddrinfo(state->addrlist); - freeaddrinfo(state->myaddrlist); - if (async) { CLEAR_BITS(state->flags, TCP_ASYNC_CONNECT); TcpWatchProc(state, state->filehandlers); @@ -1010,11 +1007,6 @@ out: 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; } } @@ -1088,6 +1080,7 @@ Tcl_OpenTcpClient( * Create a new client socket and wrap it in a channel. */ if (CreateClientSocket(interp, state) != TCL_OK) { + TcpCloseProc(state, NULL); return NULL; } -- cgit v0.12 From f5123e0b70f74ddc3f0521870aa1c318aff0aef6 Mon Sep 17 00:00:00 2001 From: max Date: Tue, 7 Jun 2011 14:59:54 +0000 Subject: Simplify file descriptor handling for client sockets and derived server sockets by putting an instance of TcpFdList into TcpState instead of just a pointer. Now only server sockets that listen on multiple addresses need the linked list of file descriptors. --- unix/tclUnixSock.c | 80 ++++++++++++++++++++++++++---------------------------- 1 file changed, 38 insertions(+), 42 deletions(-) diff --git a/unix/tclUnixSock.c b/unix/tclUnixSock.c index 39fb375..a883e8c 100644 --- a/unix/tclUnixSock.c +++ b/unix/tclUnixSock.c @@ -50,7 +50,7 @@ typedef struct TcpFdList { struct TcpState { Tcl_Channel channel; /* Channel associated with this file. */ - TcpFdList *fds; /* The file descriptors of the sockets. */ + TcpFdList fds; /* The file descriptors of the sockets. */ int flags; /* ORed combination of the bitfields defined * below. */ /* @@ -348,7 +348,7 @@ TcpBlockModeProc( } else { SET_BITS(statePtr->flags, TCP_ASYNC_SOCKET); } - if (TclUnixSetBlockingMode(statePtr->fds->fd, mode) < 0) { + if (TclUnixSetBlockingMode(statePtr->fds.fd, mode) < 0) { return errno; } return 0; @@ -390,7 +390,7 @@ WaitForConnect( timeOut = -1; } errno = 0; - state = TclUnixWaitForFile(statePtr->fds->fd, + state = TclUnixWaitForFile(statePtr->fds.fd, TCL_WRITABLE | TCL_EXCEPTION, timeOut); if (state & TCL_EXCEPTION) { return -1; @@ -443,7 +443,7 @@ TcpInputProc( if (WaitForConnect(statePtr, errorCodePtr) != 0) { return -1; } - bytesRead = recv(statePtr->fds->fd, buf, (size_t) bufSize, 0); + bytesRead = recv(statePtr->fds.fd, buf, (size_t) bufSize, 0); if (bytesRead > -1) { return bytesRead; } @@ -493,7 +493,7 @@ TcpOutputProc( if (WaitForConnect(statePtr, errorCodePtr) != 0) { return -1; } - written = send(statePtr->fds->fd, buf, (size_t) toWrite, 0); + written = send(statePtr->fds.fd, buf, (size_t) toWrite, 0); if (written > -1) { return written; } @@ -537,13 +537,15 @@ TcpCloseProc( * that called this function, so we do not have to delete them here. */ - for (fds = statePtr->fds; fds != NULL; fds = statePtr->fds) { - statePtr->fds = fds->next; + for (fds = &statePtr->fds; fds != NULL; fds = fds->next) { Tcl_DeleteFileHandler(fds->fd); if (close(fds->fd) < 0) { errorCode = errno; } - ckfree(fds); + + } + for (fds = statePtr->fds.next; fds != NULL; fds = fds->next) { + ckfree(fds); } if (statePtr->addrlist != NULL) { freeaddrinfo(statePtr->addrlist); @@ -600,7 +602,7 @@ TcpClose2Proc( } return TCL_ERROR; } - if (shutdown(statePtr->fds->fd,sd) < 0) { + if (shutdown(statePtr->fds.fd,sd) < 0) { errorCode = errno; } @@ -654,7 +656,7 @@ TcpGetOptionProc( int err, ret; if (statePtr->status == 0) { - ret = getsockopt(statePtr->fds->fd, SOL_SOCKET, SO_ERROR, + ret = getsockopt(statePtr->fds.fd, SOL_SOCKET, SO_ERROR, (char *)&err, &optlen); if (ret < 0) { err = errno; @@ -679,7 +681,7 @@ TcpGetOptionProc( address peername; socklen_t size = sizeof(peername); - if (getpeername(statePtr->fds->fd, &peername.sa, &size) >= 0) { + if (getpeername(statePtr->fds.fd, &peername.sa, &size) >= 0) { if (len == 0) { Tcl_DStringAppendElement(dsPtr, "-peername"); Tcl_DStringStartSublist(dsPtr); @@ -726,7 +728,7 @@ TcpGetOptionProc( Tcl_DStringAppendElement(dsPtr, "-sockname"); Tcl_DStringStartSublist(dsPtr); } - for (fds = statePtr->fds; fds != NULL; fds = fds->next) { + for (fds = &statePtr->fds; fds != NULL; fds = fds->next) { size = sizeof(sockname); if (getsockname(fds->fd, &(sockname.sa), &size) >= 0) { int flags = reverseDNS; @@ -817,11 +819,11 @@ TcpWatchProc( * need to cache this request until the connection has succeeded. */ statePtr->filehandlers = mask; } else if (mask) { - Tcl_CreateFileHandler(statePtr->fds->fd, mask, + Tcl_CreateFileHandler(statePtr->fds.fd, mask, (Tcl_FileProc *) Tcl_NotifyChannel, (ClientData) statePtr->channel); } else { - Tcl_DeleteFileHandler(statePtr->fds->fd); + Tcl_DeleteFileHandler(statePtr->fds.fd); } } @@ -852,7 +854,7 @@ TcpGetHandleProc( { TcpState *statePtr = (TcpState *) instanceData; - *handlePtr = INT2PTR(statePtr->fds->fd); + *handlePtr = INT2PTR(statePtr->fds.fd); return TCL_OK; } @@ -920,13 +922,13 @@ CreateClientSocket( * 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; + 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) { + state->fds.fd = socket(state->addr->ai_family, SOCK_STREAM, 0); + if (state->fds.fd < 0) { continue; } @@ -935,25 +937,25 @@ CreateClientSocket( * inherited by child processes. */ - fcntl(state->fds->fd, F_SETFD, FD_CLOEXEC); + fcntl(state->fds.fd, F_SETFD, FD_CLOEXEC); /* * Set kernel space buffering */ - TclSockMinimumBuffers(INT2PTR(state->fds->fd), SOCKET_BUFSIZE); + TclSockMinimumBuffers(INT2PTR(state->fds.fd), SOCKET_BUFSIZE); if (async) { - status = TclUnixSetBlockingMode(state->fds->fd, TCL_MODE_NONBLOCKING); + status = TclUnixSetBlockingMode(state->fds.fd, TCL_MODE_NONBLOCKING); if (status < 0) { continue; } } reuseaddr = 1; - (void) setsockopt(state->fds->fd, SOL_SOCKET, SO_REUSEADDR, + (void) setsockopt(state->fds.fd, SOL_SOCKET, SO_REUSEADDR, (char *) &reuseaddr, sizeof(reuseaddr)); - status = bind(state->fds->fd, state->myaddr->ai_addr, + status = bind(state->fds.fd, state->myaddr->ai_addr, state->myaddr->ai_addrlen); if (status < 0) { continue; @@ -966,22 +968,22 @@ CreateClientSocket( * in being informed when the connect completes. */ - status = connect(state->fds->fd, state->addr->ai_addr, + 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, + Tcl_CreateFileHandler(state->fds.fd, TCL_WRITABLE, TcpAsyncCallback, state); return TCL_OK; coro_continue: - Tcl_DeleteFileHandler(state->fds->fd); + 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, + getsockopt(state->fds.fd, SOL_SOCKET, SO_ERROR, (char *)&status, &optlen); state->status = status; } @@ -996,7 +998,7 @@ out: if (async) { CLEAR_BITS(state->flags, TCP_ASYNC_CONNECT); TcpWatchProc(state, state->filehandlers); - TclUnixSetBlockingMode(state->fds->fd, TCL_MODE_BLOCKING); + TclUnixSetBlockingMode(state->fds.fd, TCL_MODE_BLOCKING); } if (status < 0) { @@ -1072,9 +1074,7 @@ Tcl_OpenTcpClient( 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; + state->fds.fd = -1; /* * Create a new client socket and wrap it in a channel. @@ -1147,9 +1147,7 @@ TclpMakeTcpClientChannelMode( 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->fds.fd = PTR2INT(sock); statePtr->flags = 0; sprintf(channelName, SOCK_TEMPLATE, (long)statePtr); @@ -1275,8 +1273,6 @@ Tcl_OpenTcpServer( close(sock); continue; } - newfds = ckalloc(sizeof(TcpFdList)); - memset(newfds, (int) 0, sizeof(TcpFdList)); if (statePtr == NULL) { /* * Allocate a new TcpState for this socket. @@ -1284,11 +1280,13 @@ Tcl_OpenTcpServer( statePtr = ckalloc(sizeof(TcpState)); memset(statePtr, 0, sizeof(TcpState)); - statePtr->fds = newfds; statePtr->acceptProc = acceptProc; statePtr->acceptProcData = acceptProcData; sprintf(channelName, SOCK_TEMPLATE, (long)statePtr); + newfds = &statePtr->fds; } else { + newfds = ckalloc(sizeof(TcpFdList)); + memset(newfds, (int) 0, sizeof(TcpFdList)); fds->next = newfds; } newfds->fd = sock; @@ -1371,9 +1369,7 @@ TcpAccept( 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->fds.fd = newsock; sprintf(channelName, SOCK_TEMPLATE, (long)newSockState); newSockState->channel = Tcl_CreateChannel(&tcpChannelType, channelName, -- cgit v0.12 From a2c0c5611d68ee996777ad68e480daae28488ad9 Mon Sep 17 00:00:00 2001 From: max Date: Thu, 16 Jun 2011 15:21:10 +0000 Subject: * doc/socket.n: Document the fact that the event loop is now needed for [socket -async] * unix/tclUnixSock.c: Set up the file handler for async sockets to fire on exceptions in addition to writable state. * tests/socket.test: Improve error reporting when socket-14.2 times out. --- doc/socket.n | 23 +++++++++++++++++------ tests/socket.test | 3 +++ unix/tclUnixSock.c | 3 ++- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/doc/socket.n b/doc/socket.n index 0e427ed..0cb0595 100644 --- a/doc/socket.n +++ b/doc/socket.n @@ -71,12 +71,14 @@ port number will be chosen at random by the system software. This option will cause the client socket to be connected asynchronously. This means that the socket will be created immediately but may not yet be connected to the server, when the call to -\fBsocket\fR returns. When a \fBgets\fR or \fBflush\fR is done on the -socket before the connection attempt succeeds or fails, if the socket -is in blocking mode, the operation will wait until the connection is -completed or fails. If the socket is in nonblocking mode and a -\fBgets\fR or \fBflush\fR is done on the socket before the connection -attempt succeeds or fails, the operation returns immediately and +\fBsocket\fR returns. + +When a \fBgets\fR or \fBflush\fR is done on the socket before the +connection attempt succeeds or fails, if the socket is in blocking +mode, the operation will wait until the connection is completed or +fails. If the socket is in nonblocking mode and a \fBgets\fR or +\fBflush\fR is done on the socket before the connection attempt +succeeds or fails, the operation returns immediately and \fBfblocked\fR on the socket returns 1. Synchronous client sockets may be switched (after they have connected) to operating in asynchronous mode using: @@ -87,6 +89,15 @@ mode using: .CE .PP See the \fBchan\fR \fBconfigure\fR command for more details. + +The Tcl event loop should be running while an asynchronous connection +is in progress, because it may have to do several connection attempts +in the background. Runnig the event loop also allows you to set up a +writable channel event on the socket to get notified when the +asyncronous connection has succeeded or failed. See the \fBvwait\fR +and the \fBchan\fR comands for more details on the event loop and +channel events. + .RE .SH "SERVER SOCKETS" .PP diff --git a/tests/socket.test b/tests/socket.test index 39dc8de..85d4d6f 100644 --- a/tests/socket.test +++ b/tests/socket.test @@ -1772,6 +1772,9 @@ test socket-14.2 {[socket -async] fileevent connection refused} \ fileevent $client writable {set x [fconfigure $client -error]} set after [after 1000 {set x timeout}] vwait x + if {$x eq "timeout"} { + append x ": [fconfigure $client -error]" + } set x } -cleanup { after cancel $after diff --git a/unix/tclUnixSock.c b/unix/tclUnixSock.c index a883e8c..5ace251 100644 --- a/unix/tclUnixSock.c +++ b/unix/tclUnixSock.c @@ -971,7 +971,8 @@ CreateClientSocket( 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, + Tcl_CreateFileHandler(state->fds.fd, + TCL_WRITABLE | TCL_EXCEPTION, TcpAsyncCallback, state); return TCL_OK; -- cgit v0.12 From d63052263676891c816f2dbc51362eaa0e3dc048 Mon Sep 17 00:00:00 2001 From: max Date: Wed, 22 Jun 2011 14:21:28 +0000 Subject: complete a comment in socket.test --- tests/socket.test | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/socket.test b/tests/socket.test index 85d4d6f..e36914f 100644 --- a/tests/socket.test +++ b/tests/socket.test @@ -74,7 +74,8 @@ 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 +# here, so that OSes that don't have this problem can run the tests at full +# speed. 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 -- cgit v0.12