From 4b6ab47efdc71922068677ef074031e180f359d9 Mon Sep 17 00:00:00 2001 From: oehhar Date: Fri, 4 Apr 2014 16:27:39 +0000 Subject: Avoid multiple returns of connect errors --- win/tclWinSock.c | 146 +++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 103 insertions(+), 43 deletions(-) diff --git a/win/tclWinSock.c b/win/tclWinSock.c index 036f3b9..ed9fa32 100644 --- a/win/tclWinSock.c +++ b/win/tclWinSock.c @@ -172,7 +172,7 @@ struct TcpState { struct addrinfo *addr; /* Iterator over addrlist. */ struct addrinfo *myaddrlist;/* Local address. */ struct addrinfo *myaddr; /* Iterator over myaddrlist. */ - int status; /* Cache status of async socket. */ + int error; /* Cache status of async socket. */ int cachedBlocking; /* Cache blocking mode of async socket. */ volatile int connectError; /* Async connect error set by notifier thread. * Set by notifier thread, access must be @@ -255,8 +255,7 @@ static LRESULT CALLBACK SocketProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam); static int SocketsEnabled(void); static void TcpAccept(TcpFdList *fds, SOCKET newSocket, address addr); -static int WaitForConnect(TcpState *statePtr, int *errorCodePtr, - int noblock); +static int WaitForConnect(TcpState *statePtr, int *errorCodePtr); static int WaitForSocketEvent(TcpState *statePtr, int events, int *errorCodePtr); static void AddSocketInfoFd(TcpState *statePtr, SOCKET socket); @@ -573,8 +572,13 @@ TcpBlockModeProc( static int WaitForConnect( TcpState *statePtr, /* State of the socket. */ - int *errorCodePtr, /* Where to store errors? */ - int noblock) /* Don't wait, even for sockets in blocking mode */ + int *errorCodePtr) /* Where to store errors? + * A passed null-pointer activates background mode. + * In this case, an eventual error is stored in + * statePtr->error. + * In addition, we do never block and allow a next + * processing cycle to happen. + */ { int result; int oldMode; @@ -605,10 +609,11 @@ WaitForConnect( statePtr->readyEvents &= ~(FD_CONNECT); /* - * For blocking sockets disable async connect - * as we continue now synchoneously + * For blocking sockets and foreground processing + * disable async connect as we continue now synchoneously */ - if ( !(noblock || (statePtr->flags & TCP_ASYNC_SOCKET)) ) { + if ( errorCodePtr != NULL && + ! (statePtr->flags & TCP_ASYNC_SOCKET) ) { CLEAR_BITS(statePtr->flags, TCP_ASYNC_CONNECT); } @@ -624,13 +629,19 @@ WaitForConnect( /* Succesfully connected or async connect restarted */ if (result == TCL_OK) { if ( statePtr->flags & TCP_ASYNC_CONNECT_REENTER_PENDING ) { - *errorCodePtr = EWOULDBLOCK; + if (errorCodePtr != NULL) { + *errorCodePtr = EWOULDBLOCK; + } return -1; } return 0; } /* error case */ - *errorCodePtr = Tcl_GetErrno(); + if (errorCodePtr != NULL) { + *errorCodePtr = Tcl_GetErrno(); + } else { + statePtr->error = Tcl_GetErrno(); + } return -1; } @@ -641,7 +652,11 @@ WaitForConnect( * A non blocking socket waiting for an asyncronous connect * returns directly an error */ - if ( noblock || (statePtr->flags & TCP_ASYNC_SOCKET) ) { + if ( errorCodePtr == NULL ) { + /* Backround operation */ + return -1; + } else if (statePtr->flags & TCP_ASYNC_SOCKET) { + /* foreground operation but non blocking socket */ *errorCodePtr = EWOULDBLOCK; return -1; } @@ -715,7 +730,7 @@ TcpInputProc( * For a non blocking socket return EWOULDBLOCK if connect not terminated */ - if (WaitForConnect(statePtr, errorCodePtr, 0) != 0) { + if (WaitForConnect(statePtr, errorCodePtr) != 0) { return -1; } @@ -844,7 +859,7 @@ TcpOutputProc( * For a non blocking socket return EWOULDBLOCK if connect not terminated */ - if (WaitForConnect(statePtr, errorCodePtr, 0) != 0) { + if (WaitForConnect(statePtr, errorCodePtr) != 0) { return -1; } @@ -1184,7 +1199,7 @@ TcpGetOptionProc( char host[NI_MAXHOST], port[NI_MAXSERV]; SOCKET sock; size_t len = 0; - int errorCode; + int errorCode = 0; int reverseDNS = 0; #define SUPPRESS_RDNS_VAR "::tcl::unsupported::noReverseDNS" @@ -1202,8 +1217,11 @@ TcpGetOptionProc( return TCL_ERROR; } - /* Go one step in async connect */ - WaitForConnect(statePtr, &errorCode, 1); + /* + * Go one step in async connect + * If any error is thrown save it as backround error to report eventually below + */ + WaitForConnect(statePtr, NULL); sock = statePtr->sockets->fd; if (optionName != NULL) { @@ -1216,30 +1234,52 @@ TcpGetOptionProc( /* * Do not return any errors if async connect is running */ - if (! (statePtr->flags & TCP_ASYNC_CONNECT_REENTER_PENDING) ) { - int optlen; - int ret; - DWORD err; + if ( ! (statePtr->flags & TCP_ASYNC_CONNECT_REENTER_PENDING) ) { - /* - * Populater the err Variable with a possix error - */ - optlen = sizeof(int); - ret = TclWinGetSockOpt(sock, SOL_SOCKET, SO_ERROR, - (char *)&err, &optlen); - /* - * The error was not returned directly but should be - * taken from WSA - */ - if (ret == SOCKET_ERROR) { - err = WSAGetLastError(); - } - /* - * Return error message - */ - if (err) { - TclWinConvertError(err); - Tcl_DStringAppend(dsPtr, Tcl_ErrnoMsg(Tcl_GetErrno()), -1); + + if ( statePtr->flags & TCP_ASYNC_CONNECT_FAILED ) { + + /* + * In case of a failed async connect, eventually report the + * connect error only once. + * Do not report the system error, as this comes again and again. + */ + + if ( statePtr->error != 0 ) { + Tcl_DStringAppend(dsPtr, Tcl_ErrnoMsg(statePtr->error), -1); + statePtr->error = 0; + } + + } else { + + /* + * Report an eventual last error of the socket system + */ + + int optlen; + int ret; + DWORD err; + + /* + * Populater the err Variable with a possix error + */ + optlen = sizeof(int); + ret = TclWinGetSockOpt(sock, SOL_SOCKET, SO_ERROR, + (char *)&err, &optlen); + /* + * The error was not returned directly but should be + * taken from WSA + */ + if (ret == SOCKET_ERROR) { + err = WSAGetLastError(); + } + /* + * Return error message + */ + if (err) { + TclWinConvertError(err); + Tcl_DStringAppend(dsPtr, Tcl_ErrnoMsg(Tcl_GetErrno()), -1); + } } } return TCL_OK; @@ -2555,16 +2595,36 @@ SocketEventProc( return 1; } + /* + * Clear flag that (this) event is pending + */ + statePtr->flags &= ~SOCKET_PENDING; - /* Continue async connect if pending and ready */ + /* + * Continue async connect if pending and ready + */ + if ( statePtr->readyEvents & FD_CONNECT ) { - statePtr->readyEvents &= ~(FD_CONNECT); if ( statePtr->flags & TCP_ASYNC_CONNECT_REENTER_PENDING ) { + + /* + * Do one step and save eventual connect error + */ + + SetEvent(tsdPtr->socketListLock); + WaitForConnect(statePtr,NULL); + + } else { + + /* + * No async connect reenter pending. Just clear event. + */ + + statePtr->readyEvents &= ~(FD_CONNECT); SetEvent(tsdPtr->socketListLock); - CreateClientSocket(NULL, statePtr); - return 1; } + return 1; } /* -- cgit v0.12 From 77113c6828286012fe17288e3132811cb24f6fe3 Mon Sep 17 00:00:00 2001 From: oehhar Date: Mon, 7 Apr 2014 12:34:18 +0000 Subject: Return async connect error by first following read or write operation. --- win/tclWinSock.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/win/tclWinSock.c b/win/tclWinSock.c index ed9fa32..b05dc32 100644 --- a/win/tclWinSock.c +++ b/win/tclWinSock.c @@ -585,11 +585,22 @@ WaitForConnect( ThreadSpecificData *tsdPtr; /* + * Check if an async connect error is not jet reported. + * If yes, report it now. + */ + + if ( errorCodePtr != NULL && statePtr->error != 0 ) { + *errorCodePtr = statePtr->error; + statePtr->error = 0; + return -1; + } + + /* * Check if an async connect is running. If not return ok */ if ( !(statePtr->flags & TCP_ASYNC_CONNECT_REENTER_PENDING) ) return 0; - + /* * Be sure to disable event servicing so we are truly modal. */ -- cgit v0.12 From 6ab79205ac7597fa0c7b84ef86c9e341b99bc8b6 Mon Sep 17 00:00:00 2001 From: oehhar Date: Mon, 7 Apr 2014 15:08:31 +0000 Subject: Renamed function CreateClientSocket to TcpConnect and variable error to connectError --- win/tclWinSock.c | 51 ++++++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/win/tclWinSock.c b/win/tclWinSock.c index b05dc32..7593396 100644 --- a/win/tclWinSock.c +++ b/win/tclWinSock.c @@ -172,11 +172,10 @@ struct TcpState { struct addrinfo *addr; /* Iterator over addrlist. */ struct addrinfo *myaddrlist;/* Local address. */ struct addrinfo *myaddr; /* Iterator over myaddrlist. */ - int error; /* Cache status of async socket. */ + int connectError; /* Cache status of async socket. */ int cachedBlocking; /* Cache blocking mode of async socket. */ volatile int connectError; /* Async connect error set by notifier thread. - * Set by notifier thread, access must be - * protected by semaphore */ + * Access must be protected by semaphore */ struct TcpState *nextPtr; /* The next socket on the per-thread socket * list. */ }; @@ -193,7 +192,7 @@ struct TcpState { #define SOCKET_PENDING (1<<3) /* A message has been sent for this * socket */ #define TCP_ASYNC_CONNECT_REENTER_PENDING (1<<4) - /* CreateClientSocket was called to + /* TcpConnect was called to * process an async connect. This * flag indicates that reentry is * still pending */ @@ -246,7 +245,7 @@ static WNDCLASS windowClass; * Static routines for this file: */ -static int CreateClientSocket(Tcl_Interp *interp, +static int TcpConnect(Tcl_Interp *interp, TcpState *state); static void InitSockets(void); static TcpState * NewSocketInfo(SOCKET socket); @@ -553,10 +552,14 @@ TcpBlockModeProc( * * WaitForConnect -- * - * Wait for a connection on an asynchronously opened socket to be - * completed. In nonblocking mode, just test if the connection - * has completed without blocking. The noblock parameter allows to - * enforce nonblocking behaviour even on sockets in blocking mode. + * Check the state of an async connect process. If a connection + * attempt terminated, process it, which may finalize it or may + * start the next attempt. + * There are two modes of operation, defined by errorCodePtr: + * * non-NULL: Called by explicite read/write command. block if + * socket is blocking. Return a possible error and clear it. + * * Null: Called by a backround operation. Never block and + * save eventual error in statePtr->connectError. * * Results: * 0 if the connection has completed, -1 if still in progress @@ -574,9 +577,9 @@ WaitForConnect( TcpState *statePtr, /* State of the socket. */ int *errorCodePtr) /* Where to store errors? * A passed null-pointer activates background mode. - * In this case, an eventual error is stored in - * statePtr->error. - * In addition, we do never block and allow a next + * In this case, a possible error is stored in + * statePtr->connectError. + * In addition, we do never block and allow the next * processing cycle to happen. */ { @@ -589,15 +592,16 @@ WaitForConnect( * If yes, report it now. */ - if ( errorCodePtr != NULL && statePtr->error != 0 ) { - *errorCodePtr = statePtr->error; - statePtr->error = 0; + if ( errorCodePtr != NULL && statePtr->connectError != 0 ) { + *errorCodePtr = statePtr->connectError; + statePtr->connectError = 0; return -1; } /* * Check if an async connect is running. If not return ok */ + if ( !(statePtr->flags & TCP_ASYNC_CONNECT_REENTER_PENDING) ) return 0; @@ -632,7 +636,7 @@ WaitForConnect( SetEvent(tsdPtr->socketListLock); /* continue connect */ - result = CreateClientSocket(NULL, statePtr); + result = TcpConnect(NULL, statePtr); /* Restore event service mode */ (void) Tcl_SetServiceMode(oldMode); @@ -651,7 +655,7 @@ WaitForConnect( if (errorCodePtr != NULL) { *errorCodePtr = Tcl_GetErrno(); } else { - statePtr->error = Tcl_GetErrno(); + statePtr->connectError = Tcl_GetErrno(); } return -1; } @@ -1256,9 +1260,10 @@ TcpGetOptionProc( * Do not report the system error, as this comes again and again. */ - if ( statePtr->error != 0 ) { - Tcl_DStringAppend(dsPtr, Tcl_ErrnoMsg(statePtr->error), -1); - statePtr->error = 0; + if ( statePtr->connectError != 0 ) { + Tcl_DStringAppend(dsPtr, + Tcl_ErrnoMsg(statePtr->connectError), -1); + statePtr->connectError = 0; } } else { @@ -1558,7 +1563,7 @@ TcpGetHandleProc( /* *---------------------------------------------------------------------- * - * CreateClientSocket -- + * TcpConnect -- * * This function opens a new socket in client mode. * @@ -1592,7 +1597,7 @@ TcpGetHandleProc( */ static int -CreateClientSocket( +TcpConnect( Tcl_Interp *interp, /* For error reporting; can be NULL. */ TcpState *statePtr) { @@ -1904,7 +1909,7 @@ Tcl_OpenTcpClient( /* * Create a new client socket and wrap it in a channel. */ - if (CreateClientSocket(interp, statePtr) != TCL_OK) { + if (TcpConnect(interp, statePtr) != TCL_OK) { TcpCloseProc(statePtr, NULL); return NULL; } -- cgit v0.12 From f83f7d140e5150b79aa714c448879448d51f5bf5 Mon Sep 17 00:00:00 2001 From: max Date: Mon, 7 Apr 2014 15:11:32 +0000 Subject: Rename CreateClientSocket to TcpConnect --- unix/tclUnixSock.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/unix/tclUnixSock.c b/unix/tclUnixSock.c index d4b7b62..f428811 100644 --- a/unix/tclUnixSock.c +++ b/unix/tclUnixSock.c @@ -110,7 +110,7 @@ struct TcpState { * Static routines for this file: */ -static int CreateClientSocket(Tcl_Interp *interp, +static int TcpConnect(Tcl_Interp *interp, TcpState *state); static void TcpAccept(ClientData data, int mask); static int TcpBlockModeProc(ClientData data, int mode); @@ -409,13 +409,13 @@ WaitForConnect( if (noblock || (statePtr->flags & TCP_ASYNC_SOCKET)) { if (TclUnixWaitForFile(statePtr->fds.fd, TCL_WRITABLE | TCL_EXCEPTION, 0) != 0) { - CreateClientSocket(NULL, statePtr); + TcpConnect(NULL, statePtr); } } else { while (statePtr->flags & TCP_ASYNC_CONNECT) { if (TclUnixWaitForFile(statePtr->fds.fd, TCL_WRITABLE | TCL_EXCEPTION, -1) != 0) { - CreateClientSocket(NULL, statePtr); + TcpConnect(NULL, statePtr); } } } @@ -936,7 +936,7 @@ TcpGetHandleProc( * * TcpAsyncCallback -- * - * Called by the event handler that CreateClientSocket sets up + * Called by the event handler that TcpConnect sets up * internally for [socket -async] to get notified when the * asyncronous connection attempt has succeeded or failed. * @@ -949,13 +949,13 @@ TcpAsyncCallback( * TCL_READABLE, TCL_WRITABLE and * TCL_EXCEPTION. */ { - CreateClientSocket(NULL, clientData); + TcpConnect(NULL, clientData); } /* *---------------------------------------------------------------------- * - * CreateClientSocket -- + * TcpConnect -- * * This function opens a new socket in client mode. * @@ -983,7 +983,7 @@ TcpAsyncCallback( */ static int -CreateClientSocket( +TcpConnect( Tcl_Interp *interp, /* For error reporting; can be NULL. */ TcpState *statePtr) { @@ -1198,7 +1198,7 @@ Tcl_OpenTcpClient( /* * Create a new client socket and wrap it in a channel. */ - if (CreateClientSocket(interp, statePtr) != TCL_OK) { + if (TcpConnect(interp, statePtr) != TCL_OK) { TcpCloseProc(statePtr, NULL); return NULL; } -- cgit v0.12 From 7d53615f6beb67b4bdc2b0ad35b62c4667a1ab99 Mon Sep 17 00:00:00 2001 From: max Date: Mon, 7 Apr 2014 15:18:09 +0000 Subject: Rename error to connectError in struct TcpState. --- unix/tclUnixSock.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/unix/tclUnixSock.c b/unix/tclUnixSock.c index f428811..adc6243 100644 --- a/unix/tclUnixSock.c +++ b/unix/tclUnixSock.c @@ -73,7 +73,7 @@ struct TcpState { struct addrinfo *myaddr; /* Iterator over myaddrlist. */ int filehandlers; /* Caches FileHandlers that get set up while * an async socket is not yet connected. */ - int error; /* Cache SO_ERROR of async socket. */ + int connectError; /* Cache SO_ERROR of async socket. */ int cachedBlocking; /* Cache blocking mode of async socket. */ }; @@ -420,7 +420,7 @@ WaitForConnect( } } } - if (statePtr->error != 0) { + if (statePtr->connectError != 0) { return -1; } else { return 0; @@ -463,8 +463,8 @@ TcpInputProc( *errorCodePtr = 0; if (WaitForConnect(statePtr, 0) != 0) { - *errorCodePtr = statePtr->error; - statePtr->error = 0; + *errorCodePtr = statePtr->connectError; + statePtr->connectError = 0; return -1; } bytesRead = recv(statePtr->fds.fd, buf, (size_t) bufSize, 0); @@ -515,8 +515,8 @@ TcpOutputProc( *errorCodePtr = 0; if (WaitForConnect(statePtr, 0) != 0) { - *errorCodePtr = statePtr->error; - statePtr->error = 0; + *errorCodePtr = statePtr->connectError; + statePtr->connectError = 0; return -1; } written = send(statePtr->fds.fd, buf, (size_t) toWrite, 0); @@ -757,9 +757,9 @@ TcpGetOptionProc( if (statePtr->flags & TCP_ASYNC_CONNECT) { /* Suppress errors as long as we are not done */ errno = 0; - } else if (statePtr->error != 0) { - errno = statePtr->error; - statePtr->error = 0; + } else if (statePtr->connectError != 0) { + errno = statePtr->connectError; + statePtr->connectError = 0; } else { int err; getsockopt(statePtr->fds.fd, SOL_SOCKET, SO_ERROR, @@ -1070,7 +1070,7 @@ TcpConnect( if (ret < 0 && errno == EINPROGRESS) { Tcl_CreateFileHandler(statePtr->fds.fd, TCL_WRITABLE|TCL_EXCEPTION, TcpAsyncCallback, statePtr); - statePtr->error = errno = EWOULDBLOCK; + statePtr->connectError = errno = EWOULDBLOCK; return TCL_OK; reenter: @@ -1096,7 +1096,7 @@ TcpConnect( } out: - statePtr->error = error; + statePtr->connectError = error; CLEAR_BITS(statePtr->flags, TCP_ASYNC_CONNECT); if (async_callback) { /* -- cgit v0.12 From 207329f22a7bf020db137dc4e6d9b9b82d7a4f67 Mon Sep 17 00:00:00 2001 From: oehhar Date: Tue, 8 Apr 2014 14:40:20 +0000 Subject: Changed error report logic, that an async connect error is only reported by 'fconfigure -error' and not by a possible last command terminating the async connect. The terminating command always returns "socket is not connected" on connect error. In addition, some flags were renamed: TCP_ASYNC_SOCKET to TCP_NONBLOCKING and also the new state flags. --- tests/socket.test | 12 ++--- win/tclWinPort.h | 6 +++ win/tclWinSock.c | 146 +++++++++++++++++++++++++++++++++++------------------- 3 files changed, 106 insertions(+), 58 deletions(-) diff --git a/tests/socket.test b/tests/socket.test index d36d2b3..648ade5 100644 --- a/tests/socket.test +++ b/tests/socket.test @@ -1981,10 +1981,10 @@ test socket-14.7.2 {pending [socket -async] and blocking [gets], no listener} \ -body { set sock [socket -async localhost [randport]] catch {gets $sock} x - list $x [fconfigure $sock -error] + list $x [fconfigure $sock -error] [fconfigure $sock -error] } -cleanup { close $sock - } -match glob -result {{error reading "sock*": connection refused} {}} + } -match glob -result {{error reading "sock*": socket is not connected} {connection refused} {}} test socket-14.8.0 {pending [socket -async] and nonblocking [gets], server is IPv4} \ -constraints {socket supported_inet supported_inet6} \ -setup { @@ -2046,10 +2046,10 @@ test socket-14.8.2 {pending [socket -async] and nonblocking [gets], no listener} if {[catch {gets $sock} x] || $x ne "" || ![fblocked $sock]} break after 200 } - list $x [fconfigure $sock -error] + list $x [fconfigure $sock -error] [fconfigure $sock -error] } -cleanup { close $sock - } -match glob -result {{error reading "sock*": connection refused} {}} + } -match glob -result {{error reading "sock*": socket is not connected} {connection refused} {}} test socket-14.9.0 {pending [socket -async] and blocking [puts], server is IPv4} \ -constraints {socket supported_inet supported_inet6} \ -setup { @@ -2164,7 +2164,7 @@ test socket-14.11.0 {pending [socket -async] and blocking [puts], no listener, n } -cleanup { catch {close $sock} unset x - } -result {connection refused} -returnCodes 1 + } -result {socket is not connected} -returnCodes 1 test socket-14.11.1 {pending [socket -async] and blocking [puts], no listener, flush} \ -constraints {socket supported_inet supported_inet6} \ -body { @@ -2178,7 +2178,7 @@ test socket-14.11.1 {pending [socket -async] and blocking [puts], no listener, f } -cleanup { catch {close $sock} unset x - } -result {connection refused} -returnCodes 1 + } -result {socket is not connected} -returnCodes 1 test socket-14.12 {[socket -async] background progress triggered by [fconfigure -error]} \ -constraints {socket supported_inet supported_inet6} \ -body { diff --git a/win/tclWinPort.h b/win/tclWinPort.h index 61f149b..1104b53 100644 --- a/win/tclWinPort.h +++ b/win/tclWinPort.h @@ -532,6 +532,12 @@ typedef DWORD_PTR * PDWORD_PTR; * The following defines map from standard socket names to our internal * wrappers that redirect through the winSock function table (see the * file tclWinSock.c). + * + * Warning: + * This check was useful in times of Windows98 where WinSock may + * not be available. This is not the case any more. + * This function may be removed with TCL 9.0 + * */ #define getservbyname TclWinGetServByName diff --git a/win/tclWinSock.c b/win/tclWinSock.c index 7593396..dc67c4b 100644 --- a/win/tclWinSock.c +++ b/win/tclWinSock.c @@ -174,7 +174,9 @@ struct TcpState { struct addrinfo *myaddr; /* Iterator over myaddrlist. */ int connectError; /* Cache status of async socket. */ int cachedBlocking; /* Cache blocking mode of async socket. */ - volatile int connectError; /* Async connect error set by notifier thread. + volatile int notifierConnectError; + /* Async connect error set by notifier thread. + * This error is still a windows error code. * Access must be protected by semaphore */ struct TcpState *nextPtr; /* The next socket on the per-thread socket * list. */ @@ -185,19 +187,17 @@ struct TcpState { * structure. */ -#define TCP_ASYNC_SOCKET (1<<0) /* Asynchronous socket. */ +#define TCP_NONBLOCKING (1<<0) /* Socket with non-blocking I/O */ #define TCP_ASYNC_CONNECT (1<<1) /* Async connect in progress. */ #define SOCKET_EOF (1<<2) /* A zero read happened on the * socket. */ #define SOCKET_PENDING (1<<3) /* A message has been sent for this * socket */ -#define TCP_ASYNC_CONNECT_REENTER_PENDING (1<<4) - /* TcpConnect was called to +#define TCP_ASYNC_PENDING (1<<4) /* TcpConnect was called to * process an async connect. This * flag indicates that reentry is * still pending */ -#define TCP_ASYNC_CONNECT_FAILED (1<<5) - /* An async connect finally failed */ +#define TCP_ASYNC_FAILED (1<<5) /* An async connect finally failed */ /* * The following structure is what is added to the Tcl event queue when a @@ -540,9 +540,9 @@ TcpBlockModeProc( TcpState *statePtr = instanceData; if (mode == TCL_MODE_NONBLOCKING) { - statePtr->flags |= TCP_ASYNC_SOCKET; + statePtr->flags |= TCP_NONBLOCKING; } else { - statePtr->flags &= ~(TCP_ASYNC_SOCKET); + statePtr->flags &= ~(TCP_NONBLOCKING); } return 0; } @@ -554,12 +554,19 @@ TcpBlockModeProc( * * Check the state of an async connect process. If a connection * attempt terminated, process it, which may finalize it or may - * start the next attempt. + * start the next attempt. If a connect error occures, it is saved + * in statePtr->connectError to be reported by 'fconfigure -error'. + * * There are two modes of operation, defined by errorCodePtr: * * non-NULL: Called by explicite read/write command. block if - * socket is blocking. Return a possible error and clear it. - * * Null: Called by a backround operation. Never block and - * save eventual error in statePtr->connectError. + * socket is blocking. + * May return two error codes: + * * EWOULDBLOCK: if connect is still in progress + * * ENOTCONN: if connect failed. This would be the error + * message of a rect or sendto syscall so this is + * emulated here. + * * Null: Called by a backround operation. Do not block and + * don't return any error code. * * Results: * 0 if the connection has completed, -1 if still in progress @@ -577,10 +584,6 @@ WaitForConnect( TcpState *statePtr, /* State of the socket. */ int *errorCodePtr) /* Where to store errors? * A passed null-pointer activates background mode. - * In this case, a possible error is stored in - * statePtr->connectError. - * In addition, we do never block and allow the next - * processing cycle to happen. */ { int result; @@ -588,21 +591,21 @@ WaitForConnect( ThreadSpecificData *tsdPtr; /* - * Check if an async connect error is not jet reported. - * If yes, report it now. + * Check if an async connect failed already and error reporting is demanded, + * return the error ENOTCONN */ - if ( errorCodePtr != NULL && statePtr->connectError != 0 ) { - *errorCodePtr = statePtr->connectError; - statePtr->connectError = 0; + if ( errorCodePtr != NULL && + (statePtr->flags & TCP_ASYNC_FAILED) ) { + *errorCodePtr = ENOTCONN; return -1; } /* * Check if an async connect is running. If not return ok */ - - if ( !(statePtr->flags & TCP_ASYNC_CONNECT_REENTER_PENDING) ) + + if ( !(statePtr->flags & TCP_ASYNC_PENDING) ) return 0; /* @@ -611,6 +614,10 @@ WaitForConnect( oldMode = Tcl_SetServiceMode(TCL_SERVICE_NONE); + /* + * Loop in the blocking case until the connect signal is present + */ + while (1) { /* get statePtr lock */ @@ -628,22 +635,32 @@ WaitForConnect( * disable async connect as we continue now synchoneously */ if ( errorCodePtr != NULL && - ! (statePtr->flags & TCP_ASYNC_SOCKET) ) { + ! (statePtr->flags & TCP_NONBLOCKING) ) { CLEAR_BITS(statePtr->flags, TCP_ASYNC_CONNECT); } /* Free list lock */ SetEvent(tsdPtr->socketListLock); - /* continue connect */ + /* + * Continue connect. + * If switched to synchroneous connect, the connect is terminated. + */ result = TcpConnect(NULL, statePtr); /* Restore event service mode */ (void) Tcl_SetServiceMode(oldMode); - /* Succesfully connected or async connect restarted */ + /* + * Check for Succesfull connect or async connect restart + */ + if (result == TCL_OK) { - if ( statePtr->flags & TCP_ASYNC_CONNECT_REENTER_PENDING ) { + /* + * Check for async connect restart + * (not possible for foreground blocking operation) + */ + if ( statePtr->flags & TCP_ASYNC_PENDING ) { if (errorCodePtr != NULL) { *errorCodePtr = EWOULDBLOCK; } @@ -651,11 +668,14 @@ WaitForConnect( } return 0; } - /* error case */ + + /* + * Connect finally failed. + * For foreground operation return ENOTCONN. + */ + if (errorCodePtr != NULL) { - *errorCodePtr = Tcl_GetErrno(); - } else { - statePtr->connectError = Tcl_GetErrno(); + *errorCodePtr = ENOTCONN; } return -1; } @@ -664,14 +684,20 @@ WaitForConnect( SetEvent(tsdPtr->socketListLock); /* - * A non blocking socket waiting for an asyncronous connect - * returns directly an error + * Background operation returns with no action as there was no connect + * event */ + if ( errorCodePtr == NULL ) { - /* Backround operation */ return -1; - } else if (statePtr->flags & TCP_ASYNC_SOCKET) { - /* foreground operation but non blocking socket */ + } + + /* + * A non blocking socket waiting for an asyncronous connect + * returns directly the error EWOULDBLOCK + */ + + if (statePtr->flags & TCP_NONBLOCKING) { *errorCodePtr = EWOULDBLOCK; return -1; } @@ -803,7 +829,7 @@ TcpInputProc( * Check for error condition or underflow in non-blocking case. */ - if ((statePtr->flags & TCP_ASYNC_SOCKET) || (error != WSAEWOULDBLOCK)) { + if ((statePtr->flags & TCP_NONBLOCKING) || (error != WSAEWOULDBLOCK)) { TclWinConvertError(error); *errorCodePtr = Tcl_GetErrno(); bytesRead = -1; @@ -908,7 +934,7 @@ TcpOutputProc( error = WSAGetLastError(); if (error == WSAEWOULDBLOCK) { statePtr->readyEvents &= ~(FD_WRITE); - if (statePtr->flags & TCP_ASYNC_SOCKET) { + if (statePtr->flags & TCP_NONBLOCKING) { *errorCodePtr = EWOULDBLOCK; written = -1; break; @@ -1249,10 +1275,10 @@ TcpGetOptionProc( /* * Do not return any errors if async connect is running */ - if ( ! (statePtr->flags & TCP_ASYNC_CONNECT_REENTER_PENDING) ) { + if ( ! (statePtr->flags & TCP_ASYNC_PENDING) ) { - if ( statePtr->flags & TCP_ASYNC_CONNECT_FAILED ) { + if ( statePtr->flags & TCP_ASYNC_FAILED ) { /* * In case of a failed async connect, eventually report the @@ -1280,7 +1306,7 @@ TcpGetOptionProc( * Populater the err Variable with a possix error */ optlen = sizeof(int); - ret = TclWinGetSockOpt(sock, SOL_SOCKET, SO_ERROR, + ret = getsockopt(sock, SOL_SOCKET, SO_ERROR, (char *)&err, &optlen); /* * The error was not returned directly but should be @@ -1305,7 +1331,7 @@ TcpGetOptionProc( (strncmp(optionName, "-connecting", len) == 0)) { Tcl_DStringAppend(dsPtr, - (statePtr->flags & TCP_ASYNC_CONNECT_REENTER_PENDING) + (statePtr->flags & TCP_ASYNC_PENDING) ? "1" : "0", -1); return TCL_OK; } @@ -1645,7 +1671,7 @@ TcpConnect( /* * Reset last error from last try */ - statePtr->connectError = 0; + statePtr->notifierConnectError = 0; Tcl_SetErrno(0); statePtr->sockets->fd = socket(statePtr->myaddr->ai_family, SOCK_STREAM, 0); @@ -1747,7 +1773,7 @@ TcpConnect( /* * Remember that we jump back behind this next round */ - statePtr->flags |= TCP_ASYNC_CONNECT_REENTER_PENDING; + statePtr->flags |= TCP_ASYNC_PENDING; return TCL_OK; reenter: @@ -1757,11 +1783,11 @@ TcpConnect( * * Clear the reenter flag */ - statePtr->flags &= ~(TCP_ASYNC_CONNECT_REENTER_PENDING); + statePtr->flags &= ~(TCP_ASYNC_PENDING); /* get statePtr lock */ WaitForSingleObject(tsdPtr->socketListLock, INFINITE); /* Get signaled connect error */ - Tcl_SetErrno(statePtr->connectError); + TclWinConvertError((DWORD) statePtr->notifierConnectError); /* Clear eventual connect flag */ statePtr->selectEvents &= ~(FD_CONNECT); /* Free list lock */ @@ -1819,7 +1845,9 @@ out: /* Signal ready readable and writable events */ statePtr->readyEvents |= FD_WRITE | FD_READ; /* Flag error to event routine */ - statePtr->flags |= TCP_ASYNC_CONNECT_FAILED; + statePtr->flags |= TCP_ASYNC_FAILED; + /* Save connect error to be reported by 'fconfigure -error' */ + statePtr->connectError = Tcl_GetErrno(); /* Free list lock */ SetEvent(tsdPtr->socketListLock); } @@ -2260,6 +2288,12 @@ TcpAccept( * * Assumes socketMutex is held. * + * Warning: + * This check was useful in times of Windows98 where WinSock may + * not be available. This is not the case any more. + * This function may be removed with TCL 9.0. + * Any failures may be reported as panics. + * * Results: * None. * @@ -2393,6 +2427,11 @@ InitSockets(void) * * Check that the WinSock was successfully initialized. * + * Warning: + * This check was useful in times of Windows98 where WinSock may + * not be available. This is not the case any more. + * This function may be removed with TCL 9.0 + * * Results: * 1 if it is. * @@ -2622,7 +2661,7 @@ SocketEventProc( */ if ( statePtr->readyEvents & FD_CONNECT ) { - if ( statePtr->flags & TCP_ASYNC_CONNECT_REENTER_PENDING ) { + if ( statePtr->flags & TCP_ASYNC_PENDING ) { /* * Do one step and save eventual connect error @@ -2735,7 +2774,7 @@ SocketEventProc( * Throw the readable event if an async connect failed. */ - if ( statePtr->flags & TCP_ASYNC_CONNECT_FAILED ) { + if ( statePtr->flags & TCP_ASYNC_FAILED ) { mask |= TCL_READABLE; @@ -2928,7 +2967,7 @@ WaitForSocketEvent( } /* Exit loop if event did not occur but this is a non-blocking channel */ - if (statePtr->flags & TCP_ASYNC_SOCKET) { + if (statePtr->flags & TCP_NONBLOCKING) { *errorCodePtr = EWOULDBLOCK; result = 0; break; @@ -3122,8 +3161,7 @@ SocketProc( * connection failures. */ if (error != ERROR_SUCCESS) { - TclWinConvertError((DWORD) error); - statePtr->connectError = Tcl_GetErrno(); + statePtr->notifierConnectError = error; } } /* @@ -3203,6 +3241,10 @@ FindFDInList( * dynamically so we can run on systems that don't have the wsock32.dll. * We need wrappers for these interfaces because they are called from the * generic Tcl code. + * Those functions are exported by the stubs table. + * + * Warning: + * Those functions are depreciated and will be removed with TCL 9.0. * * Results: * As defined for each function. -- cgit v0.12 From bf345d6be59f6f513be07b6465487f137b9ac820 Mon Sep 17 00:00:00 2001 From: oehhar Date: Tue, 8 Apr 2014 15:15:31 +0000 Subject: Beautify check for async connect reentry --- win/tclWinSock.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/win/tclWinSock.c b/win/tclWinSock.c index dc67c4b..de4c519 100644 --- a/win/tclWinSock.c +++ b/win/tclWinSock.c @@ -595,8 +595,7 @@ WaitForConnect( * return the error ENOTCONN */ - if ( errorCodePtr != NULL && - (statePtr->flags & TCP_ASYNC_FAILED) ) { + if (errorCodePtr != NULL && (statePtr->flags & TCP_ASYNC_FAILED)) { *errorCodePtr = ENOTCONN; return -1; } @@ -605,8 +604,9 @@ WaitForConnect( * Check if an async connect is running. If not return ok */ - if ( !(statePtr->flags & TCP_ASYNC_PENDING) ) + if (!(statePtr->flags & TCP_ASYNC_CONNECT)) { return 0; + } /* * Be sure to disable event servicing so we are truly modal. @@ -1635,7 +1635,7 @@ TcpConnect( */ int async_connect = statePtr->flags & TCP_ASYNC_CONNECT; /* We were called by the event procedure and continue our loop */ - int async_callback = statePtr->sockets->fd != INVALID_SOCKET; + int async_callback = statePtr->flags & TCP_ASYNC_PENDING; ThreadSpecificData *tsdPtr = TclThreadDataKeyGet(&dataKey); if (async_callback) { @@ -1811,6 +1811,12 @@ out: * Socket connected or connection failed */ + /* + * Async connect terminated + */ + + CLEAR_BITS(statePtr->flags, TCP_ASYNC_CONNECT); + if ( Tcl_GetErrno() == 0 ) { /* * Succesfully connected -- cgit v0.12 From 9e4cc53c71c3d5416cb1e33bc5b47688ba631853 Mon Sep 17 00:00:00 2001 From: max Date: Tue, 8 Apr 2014 18:00:35 +0000 Subject: * Give clearer names to some of the state flags and sync them with Windows where it makes sense. * Rework WaitForConnect once more to always report ENOTCONN on I/O operations on failed async sockets. * Fix synchronous connections to a server that only listens on IPv6 (or whatever comes later in the list returned by getaddrinfo(), socket-15.*) * Fix spurious writable event on async sockets (socket-14.15). --- tests/socket.test | 34 ++++++++++++++ unix/tclUnixSock.c | 131 ++++++++++++++++++++++++++++++++++++----------------- 2 files changed, 124 insertions(+), 41 deletions(-) diff --git a/tests/socket.test b/tests/socket.test index 648ade5..5ff2109 100644 --- a/tests/socket.test +++ b/tests/socket.test @@ -2225,6 +2225,40 @@ test socket-14.14 {testing fileevent readable on failed async socket connect} -c after cancel $a1 } -result readable +test socket-14.15 {blocking read on async socket should not trigger event handlers} \ + -constraints socket -body { + set s [socket -async localhost [randport]] + set x ok + fileevent $s writable {set x fail} + catch {read $s} + set x + } -result ok + +set num 0 +foreach servip {127.0.0.1 ::1 localhost} { + foreach cliip {127.0.0.1 ::1 localhost} { + if {$servip eq $cliip || "localhost" in [list $servip $cliip]} { + set result {-result "sock*" -match glob} + } else { + set result { + -result {couldn't open socket: connection refused} + -returnCodes 1 + } + } + test socket-15.1.$num "Connect to $servip from $cliip" \ + -constraints {socket supported_inet supported_inet6} -setup { + set server [socket -server accept -myaddr $servip 0] + proc accept {s h p} { close $s } + set port [lindex [fconfigure $server -sockname] 2] + } -body { + set s [socket $cliip $port] + } -cleanup { + close $server + catch {close $s} + } {*}$result + incr num + } +} ::tcltest::cleanupTests flush stdout diff --git a/unix/tclUnixSock.c b/unix/tclUnixSock.c index adc6243..08a14d3 100644 --- a/unix/tclUnixSock.c +++ b/unix/tclUnixSock.c @@ -82,8 +82,13 @@ struct TcpState { * structure. */ -#define TCP_ASYNC_SOCKET (1<<0) /* Asynchronous socket. */ +#define TCP_NONBLOCKING (1<<0) /* Socket with non-blocking I/O */ #define TCP_ASYNC_CONNECT (1<<1) /* Async connect in progress. */ +#define TCP_ASYNC_PENDING (1<<4) /* TcpConnect was called to + * process an async connect. This + * flag indicates that reentry is + * still pending */ +#define TCP_ASYNC_FAILED (1<<5) /* An async connect finally failed */ /* * The following defines the maximum length of the listen queue. This is the @@ -128,7 +133,7 @@ static int TcpInputProc(ClientData instanceData, char *buf, static int TcpOutputProc(ClientData instanceData, const char *buf, int toWrite, int *errorCode); static void TcpWatchProc(ClientData instanceData, int mask); -static int WaitForConnect(TcpState *statePtr, int noblock); +static int WaitForConnect(TcpState *statePtr, int *errorCodePtr); /* * This structure describes the channel type structure for TCP socket @@ -364,9 +369,9 @@ TcpBlockModeProc( TcpState *statePtr = instanceData; if (mode == TCL_MODE_BLOCKING) { - CLEAR_BITS(statePtr->flags, TCP_ASYNC_SOCKET); + CLEAR_BITS(statePtr->flags, TCP_NONBLOCKING); } else { - SET_BITS(statePtr->flags, TCP_ASYNC_SOCKET); + SET_BITS(statePtr->flags, TCP_NONBLOCKING); } if (statePtr->flags & TCP_ASYNC_CONNECT) { statePtr->cachedBlocking = mode; @@ -383,48 +388,82 @@ TcpBlockModeProc( * * WaitForConnect -- * - * Wait for a connection on an asynchronously opened socket to be - * completed. In nonblocking mode, just test if the connection - * has completed without blocking. The noblock parameter allows to - * enforce nonblocking behaviour even on sockets in blocking mode. + * Check the state of an async connect process. If a connection + * attempt terminated, process it, which may finalize it or may + * start the next attempt. If a connect error occures, it is saved + * in statePtr->connectError to be reported by 'fconfigure -error'. + * + * There are two modes of operation, defined by errorCodePtr: + * * non-NULL: Called by explicite read/write command. block if + * socket is blocking. + * May return two error codes: + * * EWOULDBLOCK: if connect is still in progress + * * ENOTCONN: if connect failed. This would be the error + * message of a rect or sendto syscall so this is + * emulated here. + * * NULL: Called by a backround operation. Do not block and + * don't return any error code. * * Results: * 0 if the connection has completed, -1 if still in progress * or there is an error. * + * Side effects: + * Processes socket events off the system queue. + * May process asynchroneous connect. + * *---------------------------------------------------------------------- */ static int WaitForConnect( TcpState *statePtr, /* State of the socket. */ - int noblock) /* Don't wait, even for sockets in blocking mode */ + int *errorCodePtr) { + int timeout; + /* - * If an asynchronous connect is in progress, attempt to wait for it to - * complete before reading. + * Check if an async connect failed already and error reporting is demanded, + * return the error ENOTCONN */ - if (statePtr->flags & TCP_ASYNC_CONNECT) { - if (noblock || (statePtr->flags & TCP_ASYNC_SOCKET)) { - if (TclUnixWaitForFile(statePtr->fds.fd, - TCL_WRITABLE | TCL_EXCEPTION, 0) != 0) { - TcpConnect(NULL, statePtr); - } - } else { - while (statePtr->flags & TCP_ASYNC_CONNECT) { - if (TclUnixWaitForFile(statePtr->fds.fd, - TCL_WRITABLE | TCL_EXCEPTION, -1) != 0) { - TcpConnect(NULL, statePtr); - } - } - } + if (errorCodePtr != NULL && (statePtr->flags & TCP_ASYNC_FAILED)) { + *errorCodePtr = ENOTCONN; + return -1; + } + + /* + * Check if an async connect is running. If not return ok + */ + + if (!(statePtr->flags & TCP_ASYNC_PENDING)) { + return 0; } - if (statePtr->connectError != 0) { - return -1; + + if (errorCodePtr == NULL || (statePtr->flags & TCP_NONBLOCKING)) { + timeout = 0; } else { - return 0; + timeout = -1; + } + do { + if (TclUnixWaitForFile(statePtr->fds.fd, + TCL_WRITABLE | TCL_EXCEPTION, timeout) != 0) { + TcpConnect(NULL, statePtr); + } + /* Do this only once in the nonblocking case and repeat it until the + * socket is final when blocking */ + } while (timeout == -1 && statePtr->flags & TCP_ASYNC_CONNECT); + + if (errorCodePtr != NULL) { + if (statePtr->flags & TCP_ASYNC_PENDING) { + *errorCodePtr = EAGAIN; + return -1; + } else if (statePtr->connectError != 0) { + *errorCodePtr = ENOTCONN; + return -1; + } } + return 0; } /* @@ -462,9 +501,7 @@ TcpInputProc( int bytesRead; *errorCodePtr = 0; - if (WaitForConnect(statePtr, 0) != 0) { - *errorCodePtr = statePtr->connectError; - statePtr->connectError = 0; + if (WaitForConnect(statePtr, errorCodePtr) != 0) { return -1; } bytesRead = recv(statePtr->fds.fd, buf, (size_t) bufSize, 0); @@ -514,9 +551,7 @@ TcpOutputProc( int written; *errorCodePtr = 0; - if (WaitForConnect(statePtr, 0) != 0) { - *errorCodePtr = statePtr->connectError; - statePtr->connectError = 0; + if (WaitForConnect(statePtr, errorCodePtr) != 0) { return -1; } written = send(statePtr->fds.fd, buf, (size_t) toWrite, 0); @@ -744,7 +779,7 @@ TcpGetOptionProc( TcpState *statePtr = instanceData; size_t len = 0; - WaitForConnect(statePtr, 1); + WaitForConnect(statePtr, NULL); if (optionName != NULL) { len = strlen(optionName); @@ -888,7 +923,7 @@ TcpWatchProc( return; } - if (statePtr->flags & TCP_ASYNC_CONNECT) { + if (statePtr->flags & TCP_ASYNC_PENDING) { /* Async sockets use a FileHandler internally while connecting, so we * need to cache this request until the connection has succeeded. */ statePtr->filehandlers = mask; @@ -988,8 +1023,8 @@ TcpConnect( TcpState *statePtr) { socklen_t optlen; - int async_callback = (statePtr->addr != NULL); - int ret = -1, error = 0; + int async_callback = statePtr->flags & TCP_ASYNC_PENDING; + int ret = -1, error; int async = statePtr->flags & TCP_ASYNC_CONNECT; if (async_callback) { @@ -998,9 +1033,10 @@ TcpConnect( for (statePtr->addr = statePtr->addrlist; statePtr->addr != NULL; statePtr->addr = statePtr->addr->ai_next) { + for (statePtr->myaddr = statePtr->myaddrlist; statePtr->myaddr != NULL; statePtr->myaddr = statePtr->myaddr->ai_next) { - int reuseaddr; + int reuseaddr = 1; /* * No need to try combinations of local and remote addresses of @@ -1047,7 +1083,10 @@ TcpConnect( } } - reuseaddr = 1; + /* Gotta reset the error variable here, before we use it for the + * first time in this iteration. */ + error = 0; + (void) setsockopt(statePtr->fds.fd, SOL_SOCKET, SO_REUSEADDR, (char *) &reuseaddr, sizeof(reuseaddr)); ret = bind(statePtr->fds.fd, statePtr->myaddr->ai_addr, @@ -1070,10 +1109,12 @@ TcpConnect( if (ret < 0 && errno == EINPROGRESS) { Tcl_CreateFileHandler(statePtr->fds.fd, TCL_WRITABLE|TCL_EXCEPTION, TcpAsyncCallback, statePtr); - statePtr->connectError = errno = EWOULDBLOCK; + errno = EWOULDBLOCK; + SET_BITS(statePtr->flags, TCP_ASYNC_PENDING); return TCL_OK; reenter: + CLEAR_BITS(statePtr->flags, TCP_ASYNC_PENDING); Tcl_DeleteFileHandler(statePtr->fds.fd); /* @@ -1106,6 +1147,10 @@ out: TcpWatchProc(statePtr, statePtr->filehandlers); TclUnixSetBlockingMode(statePtr->fds.fd, statePtr->cachedBlocking); + if (error != 0) { + SET_BITS(statePtr->flags, TCP_ASYNC_FAILED); + } + /* * We need to forward the writable event that brought us here, bcasue * upon reading of getsockopt(SO_ERROR), at least some OSes clear the @@ -1115,7 +1160,11 @@ out: * the event mechanism one roundtrip through select(). */ + /* Note: disabling this for now as it causes spurious event triggering + * under Linux (see test socket-14.15). */ +#if 0 Tcl_NotifyChannel(statePtr->channel, TCL_WRITABLE); +#endif } if (error != 0) { /* -- cgit v0.12