From 86b84b6d213aa12182dd2809dbea0e5121bf9add Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 11 Jun 2014 17:24:58 +0000 Subject: Workaround the broken select() in some Linux kernels that fails to report a writable state on a socket when an error condition (or remote close) is present. Would be good to add actual test suite tests for this, but until then see demo scripts in the ticket 1758a0b603. --- unix/tclUnixChan.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 3 deletions(-) diff --git a/unix/tclUnixChan.c b/unix/tclUnixChan.c index 93bb1fe..fc3c10f 100644 --- a/unix/tclUnixChan.c +++ b/unix/tclUnixChan.c @@ -150,6 +150,7 @@ typedef struct TcpState { int fd; /* The socket itself. */ int flags; /* ORed combination of the bitfields defined * below. */ + int interest; /* Events types of interest. */ Tcl_TcpAcceptProc *acceptProc; /* Proc to call on accept. */ ClientData acceptProcData; /* The data for the accept proc. */ @@ -2198,6 +2199,30 @@ TcpGetOptionProc( */ static void +WrapNotify( + ClientData clientData, + int mask) +{ + TcpState *statePtr = (TcpState *) clientData; + int newmask = mask & statePtr->interest; + + if (newmask == 0) { + /* + * There was no overlap between the states the channel is + * interested in notifications for, and the states that are + * reported present on the file descriptor by select(). The + * only way that can happen is when the channel is interested + * in a writable condition, and only a readable state is reported + * present (see TcpWatchProc() below). In that case, signal back + * to the caller the writable state, which is really an error + * condition. + */ + newmask = TCL_WRITABLE; + } + Tcl_NotifyChannel(statePtr->channel, newmask); +} + +static void TcpWatchProc( ClientData instanceData, /* The socket state. */ int mask) /* Events of interest; an OR-ed combination of @@ -2214,9 +2239,30 @@ TcpWatchProc( if (!statePtr->acceptProc) { if (mask) { - Tcl_CreateFileHandler(statePtr->fd, mask, - (Tcl_FileProc *) Tcl_NotifyChannel, - (ClientData) statePtr->channel); + + /* + * Whether it is a bug or feature or otherwise, it is a fact + * of life that on at least some Linux kernels select() fails + * to report that a socket file descriptor is writable when + * the other end of the socket is closed. This is in contrast + * to the guarantees Tcl makes that its channels become + * writable and fire writable events on an error conditon. + * This has caused a leak of file descriptors in a state of + * background flushing. See Tcl ticket 1758a0b603. + * + * As a workaround, when our caller indicates an interest in + * writable notifications, we must tell the notifier built + * around select() that we are interested in the readable state + * of the file descriptor as well, as that is the only reliable + * means to get notified of error conditions. Then it is the + * task of WrapNotify() above to untangle the meaning of these + * channel states and report the chan events as best it can. + * We save a copy of the mask passed in to assist with that. + */ + + statePtr->interest = mask; + Tcl_CreateFileHandler(statePtr->fd, mask|TCL_READABLE, + (Tcl_FileProc *) WrapNotify, (ClientData) statePtr); } else { Tcl_DeleteFileHandler(statePtr->fd); } @@ -2404,6 +2450,7 @@ CreateSocket( if (asyncConnect) { statePtr->flags = TCP_ASYNC_CONNECT; } + statePtr->interest = 0; statePtr->fd = sock; return statePtr; @@ -2675,6 +2722,7 @@ MakeTcpClientChannelMode( statePtr = (TcpState *) ckalloc((unsigned) sizeof(TcpState)); statePtr->fd = PTR2INT(sock); statePtr->flags = 0; + statePtr->interest = 0; statePtr->acceptProc = NULL; statePtr->acceptProcData = NULL; @@ -2792,6 +2840,7 @@ TcpAccept( newSockState = (TcpState *) ckalloc((unsigned) sizeof(TcpState)); newSockState->flags = 0; + newSockState->interest = 0; newSockState->fd = newsock; newSockState->acceptProc = NULL; newSockState->acceptProcData = NULL; -- cgit v0.12 From 4e3fe86fc26ba3d4a51bf7abfbf11ff039228b19 Mon Sep 17 00:00:00 2001 From: dgp Date: Thu, 12 Jun 2014 13:36:18 +0000 Subject: Additional check for an error condition on the socket. --- unix/tclUnixChan.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/unix/tclUnixChan.c b/unix/tclUnixChan.c index fc3c10f..1d60340 100644 --- a/unix/tclUnixChan.c +++ b/unix/tclUnixChan.c @@ -249,6 +249,7 @@ static int TtySetOptionProc(ClientData instanceData, static int WaitForConnect(TcpState *statePtr, int *errorCodePtr); static Tcl_Channel MakeTcpClientChannelMode(ClientData tcpSocket, int mode); +static void WrapNotify(ClientData clientData, int mask); /* * This structure describes the channel type structure for file based IO: @@ -2215,8 +2216,13 @@ WrapNotify( * in a writable condition, and only a readable state is reported * present (see TcpWatchProc() below). In that case, signal back * to the caller the writable state, which is really an error - * condition. + * condition. As an extra check on that assumption, check for + * a non-zero value of errno before reporting an artificial + * writable state. */ + if (errno == 0) { + return; + } newmask = TCL_WRITABLE; } Tcl_NotifyChannel(statePtr->channel, newmask); -- cgit v0.12