From 1543959a9b88e49df5b1f1ba37872eeae9eabb1e Mon Sep 17 00:00:00 2001 From: max Date: Tue, 28 Jun 2011 11:32:15 +0000 Subject: * unix/tclUnixSock.c (CreateClientSocket): Fix and simplify posting of the writable fileevent at the end of an asynchronous connection attempt. Improve comments for some of the trickery around [socket -async]. [Bug 3325339] * tests/socket.test: Adjust tests to the async code changes. Add more tests for corner cases of async sockets. --- ChangeLog | 10 ++++++++ tests/socket.test | 67 +++++++++++++++++++++++++++++++++++++++++++----- unix/tclUnixSock.c | 75 ++++++++++++++++++++++++++++++++++++++++-------------- 3 files changed, 126 insertions(+), 26 deletions(-) diff --git a/ChangeLog b/ChangeLog index f6909b1..de04d25 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2011-06-28 Reinhard Max + + * unix/tclUnixSock.c (CreateClientSocket): Fix and simplify + posting of the writable fileevent at the end of an asynchronous + connection attempt. Improve comments for some of the trickery + around [socket -async]. [Bug 3325339] + + * tests/socket.test: Adjust tests to the async code changes. Add + more tests for corner cases of async sockets. + 2011-06-22 Andreas Kupries * library/platform/pkgIndex.tcl: Updated to platform 1.0.10. Added diff --git a/tests/socket.test b/tests/socket.test index 7f5c5c2..363f141 100644 --- a/tests/socket.test +++ b/tests/socket.test @@ -1751,21 +1751,23 @@ test socket-14.1 {[socket -async] fileevent while still connecting} \ } set server [socket -server accept -myaddr 127.0.0.1 0] set port [lindex [fconfigure $server -sockname] 2] + set x "" } -body { set client [socket -async localhost $port] fileevent $client writable { - lappend x [expr {[fconfigure $client -error] eq ""}] + lappend x [fconfigure $client -error] } - set after [after 1000 {set x timeout}] - vwait x - vwait x - set x + set after [after 1000 {lappend x timeout}] + while {[llength $x] < 2 && "timeout" ni $x} { + vwait x + } + lsort $x; # we only want to see both events, the order doesn't matter } -cleanup { after cancel $after close $server close $client unset x - } -result {ok 1} + } -result {{} ok} test socket-14.2 {[socket -async] fileevent connection refused} \ -constraints [list socket supported_any] \ -body { @@ -1782,7 +1784,58 @@ test socket-14.2 {[socket -async] fileevent connection refused} \ close $client unset x } -result "connection refused" - +test socket-14.3 {[socket -async] fileevent host unreachable} \ + -constraints [list socket supported_any] \ + -body { + # address from rfc5737 + set client [socket -async 192.0.2.42 [randport]] + fileevent $client writable {set x [fconfigure $client -error]} + set after [after 5000 {set x timeout}] + vwait x + if {$x eq "timeout"} { + append x ": [fconfigure $client -error]" + } + set x + } -cleanup { + after cancel $after + close $client + unset x + } -result "host is unreachable" +test socket-14.4 {[socket -async] and both, readdable and writable fileevents} \ + -constraints [list socket supported_any] \ + -setup { + proc accept {s a p} { + puts $s bye + close $s + } + set server [socket -server accept -myaddr 127.0.0.1 0] + set port [lindex [fconfigure $server -sockname] 2] + set x "" + } -body { + set client [socket -async localhost $port] + fileevent $client writable { + lappend x [fconfigure $client -error] + fileevent $client writable {} + } + fileevent $client readable {lappend x [gets $client]} + set after [after 1000 {lappend x timeout}] + while {[llength $x] < 2 && "timeout" ni $x} { + vwait x + } + lsort $x + } -cleanup { + after cancel $after + close $client + close $server + } -result {{} bye} +test socket-14.5 {[socket -async] which fails before any connect() can be made} \ + -constraints [list socket supported_any] \ + -body { + # addresses from rfc5737 + socket -async -myaddr 192.0.2.42 198.51.100.42 [randport] + } \ + -returnCodes 1 \ + -result {couldn't open socket: cannot assign requested address} ::tcltest::cleanupTests flush stdout return diff --git a/unix/tclUnixSock.c b/unix/tclUnixSock.c index 5ace251..52b089c 100644 --- a/unix/tclUnixSock.c +++ b/unix/tclUnixSock.c @@ -858,6 +858,17 @@ TcpGetHandleProc( return TCL_OK; } +/* + *---------------------------------------------------------------------- + * + * TcpAsyncCallback -- + * + * Called by the event handler that CreateClientSocket sets up + * internally for [socket -async] to get notified when the + * asyncronous connection attempt has succeeded or failed. + * + *---------------------------------------------------------------------- + */ static void TcpAsyncCallback( ClientData clientData, /* The socket state. */ @@ -883,6 +894,18 @@ TcpAsyncCallback( * Side effects: * Opens a socket. * + * Remarks: + * A single host name may resolve to more than one IP address, e.g. for + * an IPv4/IPv6 dual stack host. For handling asyncronously connecting + * sockets in the background for such hosts, this function can act as a + * coroutine. On the first call, it sets up the control variables for the + * two nested loops over the local and remote addresses. Once the first + * connection attempt is in progress, it sets up itself as a writable + * event handler for that socket, and returns. When the callback occurs, + * control is transferred to the "reenter" label, right after the initial + * return and the loops resume as if they had never been interrupted. + * For syncronously connecting sockets, the loops work the usual way. + * *---------------------------------------------------------------------- */ @@ -892,14 +915,14 @@ CreateClientSocket( TcpState *state) { socklen_t optlen; - int in_coro = (state->addr != NULL); + int async_callback = (state->addr != NULL); int status; int async = state->flags & TCP_ASYNC_CONNECT; - if (in_coro) { - goto coro_continue; + if (async_callback) { + goto reenter; } - + for (state->addr = state->addrlist; state->addr != NULL; state->addr = state->addr->ai_next) { @@ -976,12 +999,13 @@ CreateClientSocket( TcpAsyncCallback, state); return TCL_OK; - coro_continue: + reenter: 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] + * Read the error state from the socket to see if the async + * connection has succeeded or failed. As this clears the + * error condition, we cache the status in the socket state + * struct for later retrieval by [fconfigure -error]. */ optlen = sizeof(int); getsockopt(state->fds.fd, SOL_SOCKET, SO_ERROR, @@ -996,22 +1020,35 @@ CreateClientSocket( out: - if (async) { + if (async_callback) { + /* + * An asynchonous connection has finally succeeded or failed. + */ 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->channel, TCL_WRITABLE); - } else { - if (interp != NULL) { - Tcl_AppendResult(interp, "couldn't open socket: ", - Tcl_PosixError(interp), NULL); - } - return TCL_ERROR; + /* + * We need to forward the writable event that brought us here, bcasue + * upon reading of getsockopt(SO_ERROR), at least some OSes clear the + * writable state from the socket, and so a subsequent select() on + * behalf of a script level [fileevent] would not fire. It doesn't + * hurt that this is also called in the successful case and will save + * the event mechanism one roundtrip through select(). + */ + Tcl_NotifyChannel(state->channel, TCL_WRITABLE); + + } else if (status != 0) { + /* + * Failure for either a synchronous connection, or an async one that + * failed before it could enter background mode, e.g. because an + * invalid -myaddr was given. + */ + if (interp != NULL) { + Tcl_AppendResult(interp, "couldn't open socket: ", + Tcl_PosixError(interp), NULL); } + return TCL_ERROR; } return TCL_OK; } -- cgit v0.12