Attachment "td-patch-3545363-3.diff" to
ticket [3545363fff]
added by
twylite
2012-07-25 03:07:38.
Index: win/tclWinSock.c
==================================================================
--- win/tclWinSock.c
+++ win/tclWinSock.c
@@ -218,11 +218,11 @@
static SocketInfo * NewSocketInfo(SOCKET socket);
static void SocketExitHandler(ClientData clientData);
static LRESULT CALLBACK SocketProc(HWND hwnd, UINT message, WPARAM wParam,
LPARAM lParam);
static int SocketsEnabled(void);
-static void TcpAccept(TcpFdList *fds);
+static void TcpAccept(TcpFdList *fds, SOCKET newSocket, address addr);
static int WaitForSocketEvent(SocketInfo *infoPtr, int events,
int *errorCodePtr);
static DWORD WINAPI SocketThread(LPVOID arg);
static void TcpThreadActionProc(ClientData instanceData,
int action);
@@ -690,10 +690,13 @@
SocketInfo *infoPtr;
SocketEvent *eventPtr = (SocketEvent *) evPtr;
int mask = 0, events;
ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey);
TcpFdList *fds;
+ SOCKET newSocket;
+ address addr;
+ int len;
if (!(flags & TCL_FILE_EVENTS)) {
return 0;
}
@@ -706,17 +709,17 @@
infoPtr = infoPtr->nextPtr) {
if (infoPtr->sockets->fd == eventPtr->socket) {
break;
}
}
- SetEvent(tsdPtr->socketListLock);
/*
* Discard events that have gone stale.
*/
if (!infoPtr) {
+ SetEvent(tsdPtr->socketListLock);
return 1;
}
infoPtr->flags &= ~SOCKET_PENDING;
@@ -724,19 +727,72 @@
* Handle connection requests directly.
*/
if (infoPtr->readyEvents & FD_ACCEPT) {
for (fds = infoPtr->sockets; fds != NULL; fds = fds->next) {
- TcpAccept(fds);
- }
- return 1;
- }
-
- /*
- * Mask off unwanted events and compute the read/write mask so we can
- * notify the channel.
- */
+ /*
+ * Accept the incoming connection request.
+ */
+ len = sizeof(address);
+
+ newSocket = accept(fds->fd, &(addr.sa), &len);
+
+ /* On Tcl server sockets with multiple OS fds we loop over the fds trying
+ * an accept() on each, so we expect INVALID_SOCKET. There are also other
+ * network stack conditions that can result in FD_ACCEPT but a subsequent
+ * failure on accept() by the time we get around to it.
+ * Access to sockets (acceptEventCount, readyEvents) in socketList
+ * is still protected by the lock (prevents reintroduction of
+ * SF Tcl Bug 3056775.
+ */
+
+ if (newSocket == INVALID_SOCKET) {
+ /* int err = WSAGetLastError(); */
+ continue;
+ }
+
+ /*
+ * It is possible that more than one FD_ACCEPT has been sent, so an extra
+ * count must be kept. Decrement the count, and reset the readyEvent bit
+ * if the count is no longer > 0.
+ */
+ infoPtr->acceptEventCount--;
+
+ if (infoPtr->acceptEventCount <= 0) {
+ infoPtr->readyEvents &= ~(FD_ACCEPT);
+ }
+
+ SetEvent(tsdPtr->socketListLock);
+
+ /* Caution: TcpAccept() has the side-effect of evaluating the server
+ * accept script (via AcceptCallbackProc() in tclIOCmd.c), which can
+ * close the server socket and invalidate infoPtr and fds.
+ * If TcpAccept() accepts a socket we must return immediately and let
+ * SocketCheckProc queue additional FD_ACCEPT events.
+ */
+ TcpAccept(fds, newSocket, addr);
+ return 1;
+ }
+
+ /* Loop terminated with no sockets accepted; clear the ready mask so
+ * we can detect the next connection request. Note that connection
+ * requests are level triggered, so if there is a request already
+ * pending, a new event will be generated.
+ */
+ infoPtr->acceptEventCount = 0;
+ infoPtr->readyEvents &= ~(FD_ACCEPT);
+
+ SetEvent(tsdPtr->socketListLock);
+ return 1;
+ }
+
+ SetEvent(tsdPtr->socketListLock);
+
+ /*
+ * Mask off unwanted events and compute the read/write mask so we can
+ * notify the channel.
+ */
events = infoPtr->readyEvents & infoPtr->watchEvents;
if (events & FD_CLOSE) {
/*
@@ -870,13 +926,19 @@
* Clean up the OS socket handle. The default Windows setting for a
* socket is SO_DONTLINGER, which does a graceful shutdown in the
* background.
*/
- if (closesocket(infoPtr->sockets->fd) == SOCKET_ERROR) {
- TclWinConvertError((DWORD) WSAGetLastError());
- errorCode = Tcl_GetErrno();
+ while ( infoPtr->sockets != NULL ) {
+ TcpFdList *thisfd = infoPtr->sockets;
+ infoPtr->sockets = thisfd->next;
+
+ if (closesocket(thisfd->fd) == SOCKET_ERROR) {
+ TclWinConvertError((DWORD) WSAGetLastError());
+ errorCode = Tcl_GetErrno();
+ }
+ ckfree(thisfd);
}
}
/*
* TIP #218. Removed the code removing the structure from the global
@@ -932,18 +994,65 @@
"Socket close2proc called bidirectionally", NULL);
}
return TCL_ERROR;
}
+ /* single fd operation: Tcl_OpenTcpServer() does not set TCL_READABLE or
+ * TCL_WRITABLE so this should never be called for a server socket. */
if (shutdown(infoPtr->sockets->fd, sd) == SOCKET_ERROR) {
TclWinConvertError((DWORD) WSAGetLastError());
errorCode = Tcl_GetErrno();
}
return errorCode;
}
+/*
+ *----------------------------------------------------------------------
+ *
+ * AddSocketInfoFd --
+ *
+ * This function adds a SOCKET file descriptor to the 'sockets' linked
+ * list of a SocketInfo structure.
+ *
+ * Results:
+ * None.
+ *
+ * Side effects:
+ * None, except for allocation of memory.
+ *
+ *----------------------------------------------------------------------
+ */
+
+static void
+AddSocketInfoFd(
+ SocketInfo *infoPtr,
+ SOCKET socket)
+{
+ TcpFdList *fds = infoPtr->sockets;
+
+ if ( fds == NULL ) {
+ /* Add the first FD */
+ infoPtr->sockets = ckalloc(sizeof(TcpFdList));
+ fds = infoPtr->sockets;
+ } else {
+ /* Find end of list and append FD */
+ while ( fds->next != NULL ) {
+ fds = fds->next;
+ }
+
+ fds->next = ckalloc(sizeof(TcpFdList));
+ fds = fds->next;
+ }
+
+ /* Populate new FD */
+ fds->fd = socket;
+ fds->infoPtr = infoPtr;
+ fds->next = NULL;
+}
+
+
/*
*----------------------------------------------------------------------
*
* NewSocketInfo --
*
@@ -961,18 +1070,14 @@
static SocketInfo *
NewSocketInfo(
SOCKET socket)
{
SocketInfo *infoPtr = ckalloc(sizeof(SocketInfo));
- TcpFdList *fds = ckalloc(sizeof(TcpFdList));
- fds->fd = socket;
- fds->next = NULL;
- fds->infoPtr = infoPtr;
/* ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey); */
infoPtr->channel = 0;
- infoPtr->sockets = fds;
+ infoPtr->sockets = NULL;
infoPtr->flags = 0;
infoPtr->watchEvents = 0;
infoPtr->readyEvents = 0;
infoPtr->selectEvents = 0;
infoPtr->acceptEventCount = 0;
@@ -985,10 +1090,12 @@
* list. This is now handled in the thread action callbacks, and only
* there.
*/
infoPtr->nextPtr = NULL;
+
+ AddSocketInfoFd(infoPtr, socket);
return infoPtr;
}
/*
@@ -1055,11 +1162,10 @@
&errorMsg)) {
goto error;
}
if (server) {
- TcpFdList *fds = NULL, *newfds;
for (addrPtr = addrlist; addrPtr != NULL; addrPtr = addrPtr->ai_next) {
sock = socket(addrPtr->ai_family, SOCK_STREAM, 0);
if (sock == INVALID_SOCKET) {
TclWinConvertError((DWORD) WSAGetLastError());
@@ -1138,27 +1244,20 @@
/*
* Add this socket to the global list of sockets.
*/
infoPtr = NewSocketInfo(sock);
- fds = infoPtr->sockets;
/*
* Set up the select mask for connection request events.
*/
infoPtr->selectEvents = FD_ACCEPT;
infoPtr->watchEvents |= FD_ACCEPT;
} else {
- newfds = ckalloc(sizeof(TcpFdList));
- memset(newfds, (int) 0, sizeof(TcpFdList));
- newfds->fd = sock;
- newfds->infoPtr = infoPtr;
- newfds->next = NULL;
- fds->next = newfds;
- fds = newfds;
+ AddSocketInfoFd( infoPtr, sock );
}
}
} else {
for (addrPtr = addrlist; addrPtr != NULL;
addrPtr = addrPtr->ai_next) {
@@ -1535,12 +1634,13 @@
/*
*----------------------------------------------------------------------
*
* TcpAccept --
*
- * Accept a TCP socket connection. This is called by SocketEventProc and
- * it in turns calls the registered accept function.
+ * Creates a channel for a newly accepted socket connection. This is
+ * called by SocketEventProc and it in turns calls the registered
+ * accept function.
*
* Results:
* None.
*
* Side effects:
@@ -1549,64 +1649,21 @@
*----------------------------------------------------------------------
*/
static void
TcpAccept(
- TcpFdList *fds) /* Socket to accept. */
+ TcpFdList *fds, /* Server socket that accepted newSocket. */
+ SOCKET newSocket, /* Newly accepted socket. */
+ address addr) /* Address of new socket. */
{
- SOCKET newSocket;
SocketInfo *newInfoPtr;
SocketInfo *infoPtr = fds->infoPtr;
- address addr;
- int len;
+ int len = sizeof(addr);
char channelName[16 + TCL_INTEGER_SPACE];
char host[NI_MAXHOST], port[NI_MAXSERV];
ThreadSpecificData *tsdPtr = TclThreadDataKeyGet(&dataKey);
- /*
- * Accept the incoming connection request.
- */
-
- len = sizeof(address);
-
- newSocket = accept(fds->fd, &(addr.sa), &len);
-
- /*
- * Protect access to sockets (acceptEventCount, readyEvents) in socketList
- * by the lock. Fix for SF Tcl Bug 3056775.
- */
-
- WaitForSingleObject(tsdPtr->socketListLock, INFINITE);
-
- /*
- * Clear the ready mask so we can detect the next connection request. Note
- * that connection requests are level triggered, so if there is a request
- * already pending, a new event will be generated.
- */
-
- if (newSocket == INVALID_SOCKET) {
- infoPtr->acceptEventCount = 0;
- infoPtr->readyEvents &= ~(FD_ACCEPT);
-
- SetEvent(tsdPtr->socketListLock);
- return;
- }
-
- /*
- * It is possible that more than one FD_ACCEPT has been sent, so an extra
- * count must be kept. Decrement the count, and reset the readyEvent bit
- * if the count is no longer > 0.
- */
-
- infoPtr->acceptEventCount--;
-
- if (infoPtr->acceptEventCount <= 0) {
- infoPtr->readyEvents &= ~(FD_ACCEPT);
- }
-
- SetEvent(tsdPtr->socketListLock);
-
/*
* Win-NT has a misfeature that sockets are inherited in child processes
* by default. Turn off the inherit bit.
*/
@@ -1646,11 +1703,11 @@
if (infoPtr->acceptProc != NULL) {
getnameinfo(&(addr.sa), len, host, sizeof(host), port, sizeof(port),
NI_NUMERICHOST|NI_NUMERICSERV);
infoPtr->acceptProc(infoPtr->acceptProcData, newInfoPtr->channel,
- host, atoi(port));
+ host, atoi(port));
}
}
/*
*----------------------------------------------------------------------
@@ -1721,10 +1778,11 @@
*/
while (1) {
SendMessage(tsdPtr->hwnd, SOCKET_SELECT,
(WPARAM) UNSELECT, (LPARAM) infoPtr);
+ /* single fd operation: this proc is only called for a connected socket. */
bytesRead = recv(infoPtr->sockets->fd, buf, toRead, 0);
infoPtr->readyEvents &= ~(FD_READ);
/*
* Check for end-of-file condition or successful read.
@@ -1841,10 +1899,11 @@
while (1) {
SendMessage(tsdPtr->hwnd, SOCKET_SELECT,
(WPARAM) UNSELECT, (LPARAM) infoPtr);
+ /* single fd operation: this proc is only called for a connected socket. */
bytesWritten = send(infoPtr->sockets->fd, buf, toWrite, 0);
if (bytesWritten != SOCKET_ERROR) {
/*
* Since Windows won't generate a new write event until we hit an
* overflow condition, we need to force the event loop to poll
@@ -1936,10 +1995,11 @@
}
return TCL_ERROR;
}
#ifdef TCL_FEATURE_KEEPALIVE_NAGLE
+ #error "TCL_FEATURE_KEEPALIVE_NAGLE not reviewed for whether to treat infoPtr->sockets as single fd or list"
sock = infoPtr->sockets->fd;
if (!strcasecmp(optionName, "-keepalive")) {
BOOL val = FALSE;
int boolVar, rtn;
@@ -2399,10 +2459,11 @@
LPARAM lParam)
{
int event, error;
SOCKET socket;
SocketInfo *infoPtr;
+ TcpFdList *fds = NULL;
ThreadSpecificData *tsdPtr = (ThreadSpecificData *)
#ifdef _WIN64
GetWindowLongPtr(hwnd, GWLP_USERDATA);
#else
GetWindowLong(hwnd, GWL_USERDATA);
@@ -2443,78 +2504,83 @@
*/
WaitForSingleObject(tsdPtr->socketListLock, INFINITE);
for (infoPtr = tsdPtr->socketList; infoPtr != NULL;
infoPtr = infoPtr->nextPtr) {
- if (infoPtr->sockets->fd == socket) {
- /*
- * Update the socket state.
- *
- * A count of FD_ACCEPTS is stored, so if an FD_CLOSE event
- * happens, then clear the FD_ACCEPT count. Otherwise,
- * increment the count if the current event is an FD_ACCEPT.
- */
-
- if (event & FD_CLOSE) {
- infoPtr->acceptEventCount = 0;
- infoPtr->readyEvents &= ~(FD_WRITE|FD_ACCEPT);
- } else if (event & FD_ACCEPT) {
- infoPtr->acceptEventCount++;
- }
-
- if (event & FD_CONNECT) {
+ for (fds = infoPtr->sockets; fds != NULL; fds = fds->next) {
+ if (fds->fd == socket) {
/*
- * The socket is now connected, clear the async connect
- * flag.
+ * Update the socket state.
+ *
+ * A count of FD_ACCEPTS is stored, so if an FD_CLOSE event
+ * happens, then clear the FD_ACCEPT count. Otherwise,
+ * increment the count if the current event is an FD_ACCEPT.
*/
- infoPtr->flags &= ~(SOCKET_ASYNC_CONNECT);
+ if (event & FD_CLOSE) {
+ infoPtr->acceptEventCount = 0;
+ infoPtr->readyEvents &= ~(FD_WRITE|FD_ACCEPT);
+ } else if (event & FD_ACCEPT) {
+ infoPtr->acceptEventCount++;
+ }
+
+ if (event & FD_CONNECT) {
+ /*
+ * The socket is now connected, clear the async connect
+ * flag.
+ */
+
+ infoPtr->flags &= ~(SOCKET_ASYNC_CONNECT);
+
+ /*
+ * Remember any error that occurred so we can report
+ * connection failures.
+ */
+
+ if (error != ERROR_SUCCESS) {
+ TclWinConvertError((DWORD) error);
+ infoPtr->lastError = Tcl_GetErrno();
+ }
+ }
+
+ if (infoPtr->flags & SOCKET_ASYNC_CONNECT) {
+ infoPtr->flags &= ~(SOCKET_ASYNC_CONNECT);
+ if (error != ERROR_SUCCESS) {
+ TclWinConvertError((DWORD) error);
+ infoPtr->lastError = Tcl_GetErrno();
+ }
+ infoPtr->readyEvents |= FD_WRITE;
+ }
+ infoPtr->readyEvents |= event;
/*
- * Remember any error that occurred so we can report
- * connection failures.
+ * Wake up the Main Thread.
*/
- if (error != ERROR_SUCCESS) {
- TclWinConvertError((DWORD) error);
- infoPtr->lastError = Tcl_GetErrno();
- }
+ SetEvent(tsdPtr->readyEvent);
+ Tcl_ThreadAlert(tsdPtr->threadId);
+ break;
}
-
- if (infoPtr->flags & SOCKET_ASYNC_CONNECT) {
- infoPtr->flags &= ~(SOCKET_ASYNC_CONNECT);
- if (error != ERROR_SUCCESS) {
- TclWinConvertError((DWORD) error);
- infoPtr->lastError = Tcl_GetErrno();
- }
- infoPtr->readyEvents |= FD_WRITE;
- }
- infoPtr->readyEvents |= event;
-
- /*
- * Wake up the Main Thread.
- */
-
- SetEvent(tsdPtr->readyEvent);
- Tcl_ThreadAlert(tsdPtr->threadId);
- break;
}
}
SetEvent(tsdPtr->socketListLock);
break;
case SOCKET_SELECT:
infoPtr = (SocketInfo *) lParam;
- if (wParam == SELECT) {
- WSAAsyncSelect(infoPtr->sockets->fd, hwnd,
- SOCKET_MESSAGE, infoPtr->selectEvents);
- } else {
- /*
- * Clear the selection mask
- */
+ for (fds = infoPtr->sockets; fds != NULL; fds = fds->next) {
+ infoPtr = (SocketInfo *) lParam;
+ if (wParam == SELECT) {
+ WSAAsyncSelect(fds->fd, hwnd,
+ SOCKET_MESSAGE, infoPtr->selectEvents);
+ } else {
+ /*
+ * Clear the selection mask
+ */
- WSAAsyncSelect(infoPtr->sockets->fd, hwnd, 0, 0);
+ WSAAsyncSelect(fds->fd, hwnd, 0, 0);
+ }
}
break;
case SOCKET_TERMINATE:
DestroyWindow(hwnd);
