summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authortwylite <twylite@crypt.co.za>2012-07-24 13:58:44 (GMT)
committertwylite <twylite@crypt.co.za>2012-07-24 13:58:44 (GMT)
commite574cc448ea3a74b469753686dbe9e2c5ac90037 (patch)
tree197f430c7bd9fde3b51763c25fcaab3940ef9303
parente47ba9f0c364c577be35a2cc155d90078742a2d7 (diff)
downloadtcl-e574cc448ea3a74b469753686dbe9e2c5ac90037.zip
tcl-e574cc448ea3a74b469753686dbe9e2c5ac90037.tar.gz
tcl-e574cc448ea3a74b469753686dbe9e2c5ac90037.tar.bz2
[Bug: 3545363]: Handle socket with multiple underlying file descriptors where required (TcpCloseProc, SocketProc). Refactor socket/descriptor setup. Fix memory leak in socket close (TcpCloseProc) and related dangling pointers in SocketEventProc.
-rw-r--r--win/tclWinSock.c307
1 files changed, 187 insertions, 120 deletions
diff --git a/win/tclWinSock.c b/win/tclWinSock.c
index 5603ef3..c651deb 100644
--- a/win/tclWinSock.c
+++ b/win/tclWinSock.c
@@ -220,7 +220,7 @@ 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);
@@ -692,6 +692,9 @@ SocketEventProc(
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;
@@ -708,13 +711,13 @@ SocketEventProc(
break;
}
}
- SetEvent(tsdPtr->socketListLock);
/*
* Discard events that have gone stale.
*/
if (!infoPtr) {
+ SetEvent(tsdPtr->socketListLock);
return 1;
}
@@ -726,11 +729,65 @@ SocketEventProc(
if (infoPtr->readyEvents & FD_ACCEPT) {
for (fds = infoPtr->sockets; fds != NULL; fds = fds->next) {
- TcpAccept(fds);
+
+ /*
+ * 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.
@@ -872,9 +929,15 @@ TcpCloseProc(
* 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);
}
}
@@ -934,6 +997,8 @@ TcpClose2Proc(
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();
@@ -945,6 +1010,51 @@ TcpClose2Proc(
/*
*----------------------------------------------------------------------
*
+ * 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 --
*
* This function allocates and initializes a new SocketInfo structure.
@@ -963,14 +1073,10 @@ 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;
@@ -988,6 +1094,8 @@ NewSocketInfo(
infoPtr->nextPtr = NULL;
+ AddSocketInfoFd(infoPtr, socket);
+
return infoPtr;
}
@@ -1057,7 +1165,6 @@ CreateSocket(
}
if (server) {
- TcpFdList *fds = NULL, *newfds;
for (addrPtr = addrlist; addrPtr != NULL; addrPtr = addrPtr->ai_next) {
sock = socket(addrPtr->ai_family, SOCK_STREAM, 0);
@@ -1140,7 +1247,6 @@ CreateSocket(
*/
infoPtr = NewSocketInfo(sock);
- fds = infoPtr->sockets;
/*
* Set up the select mask for connection request events.
@@ -1150,13 +1256,7 @@ CreateSocket(
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 {
@@ -1537,8 +1637,9 @@ Tcl_OpenTcpServer(
*
* 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.
@@ -1551,61 +1652,18 @@ Tcl_OpenTcpServer(
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.
*/
@@ -1648,7 +1706,7 @@ TcpAccept(
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));
}
}
@@ -1723,6 +1781,7 @@ TcpInputProc(
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);
@@ -1843,6 +1902,7 @@ TcpOutputProc(
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) {
/*
@@ -1938,6 +1998,7 @@ TcpSetOptionProc(
}
#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")) {
@@ -2401,6 +2462,7 @@ SocketProc(
int event, error;
SOCKET socket;
SocketInfo *infoPtr;
+ TcpFdList *fds = NULL;
ThreadSpecificData *tsdPtr = (ThreadSpecificData *)
#ifdef _WIN64
GetWindowLongPtr(hwnd, GWLP_USERDATA);
@@ -2445,58 +2507,60 @@ SocketProc(
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++;
+ }
- /*
- * Remember any error that occurred so we can report
- * connection failures.
- */
+ if (event & FD_CONNECT) {
+ /*
+ * The socket is now connected, clear the async connect
+ * flag.
+ */
+
+ infoPtr->flags &= ~(SOCKET_ASYNC_CONNECT);
- if (error != ERROR_SUCCESS) {
- TclWinConvertError((DWORD) error);
- infoPtr->lastError = Tcl_GetErrno();
+ /*
+ * 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();
+ 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 |= FD_WRITE;
- }
- infoPtr->readyEvents |= event;
+ infoPtr->readyEvents |= event;
- /*
- * Wake up the Main Thread.
- */
+ /*
+ * Wake up the Main Thread.
+ */
- SetEvent(tsdPtr->readyEvent);
- Tcl_ThreadAlert(tsdPtr->threadId);
- break;
+ SetEvent(tsdPtr->readyEvent);
+ Tcl_ThreadAlert(tsdPtr->threadId);
+ break;
+ }
}
}
SetEvent(tsdPtr->socketListLock);
@@ -2504,15 +2568,18 @@ SocketProc(
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;