#3416 Unix Notifier thread causes a number of problems.

[Anonymous]

The design of the Unix notifier as a separate thread causes a number
of problems such as bad interactions with [fork] and lost signal
delivery.

Included is a sample Notifier that does not use a separate thread.

/*
* Notify.c --
*/

#include "tclInt.h"
#include "tclPort.h"
#include <sys/types.h>
#include <sys/poll.h>
#include <stdio.h>
#include <signal.h>

#include <unistd.h>
#include <sys/wait.h>
#include <errno.h>
/*
* This structure is used to keep track of the notifier info for a
* a registered file.
*/

typedef struct FileHandler {
int fd;
int mask; /* Mask of desired events: TCL_READABLE,
* etc. */
int readyMask; /* Mask of events that have been seen since the
* last time file handlers were invoked for
* this file. */
Tcl_FileProc *proc; /* Procedure to call, in the style of
* Tcl_CreateFileHandler. */
ClientData clientData; /* Argument to pass to proc. */
struct FileHandler *nextPtr;/* Next in list of all files we care about. */
} FileHandler;

/*
* The following structure is what is added to the Tcl event queue when
* file handlers are ready to fire.
*/

typedef struct FileHandlerEvent {
Tcl_Event header; /* Information that is standard for
* all events. */
int fd; /* File descriptor that is ready. Used
* to find the FileHandler structure for
* the file (can't point directly to the
* FileHandler structure because it could
* go away while the event is queued). */
} FileHandlerEvent;

/*
* The following static structure contains the state information for the
* poll-based implementation of the Tcl notifier. One of these structures
* is created for each thread that is using the notifier.
*/

typedef struct ThreadSpecificData {
FileHandler *firstFileHandlerPtr;
/* Pointer to head of file handler list. */
struct pollfd *pollargs; /* array of args for poll function */
unsigned maxfds, numfds; /* size of pollargs, number elements used */
#ifdef TCL_THREADS
Tcl_ThreadId self; /* process id for signal */
int notified; /* if true, Tcl_AlertNotifier was called */
int running; /* if true, thread will check for incoming */
/* events before blocking. This is used */
/* in Tcl_AlertNotifier as an optimization */
#endif
} ThreadSpecificData;

extern TclStubs tclStubs;

static Tcl_ThreadDataKey dataKey;

#ifdef TCL_THREADS

/*
* The notifierMutex locks access to all of the global notifier state.
*/

TCL_DECLARE_MUTEX(notifierMutex)

static int numThreads;

#endif

/*
* Static routines defined in this file.
*/

static int FileHandlerEventProc _ANSI_ARGS_((Tcl_Event *evPtr,
int flags));

/*
*----------------------------------------------------------------------
*
* defaultSIGCHLDHandler --
*
* This signal handler provides the SIG_IGN semantics of Unix
* SIGCHLD signals as an explicit handler; ie it harvests
* zombies.
*
* This handler is set only for its side effects, that is, we
* don't really care about the status of child processes, but
* we do want to use kill(SIGCHLD) to interrupt blocked
* system calls.
*
* No harm is done if this handler is replaced by another
* handler (for example, by an extension): any handler is
* acceptable, but no handler is not!
*
* Results:
* Zombies are harvested.
*
* Side effects:
* Certain "slow" system calls (like read, select) will be
* interrupted with errno set to EINTR after this handler has
* been invoked. This will not happen if the SIGCHLD handler
* is set to SIG_DFL
*
*----------------------------------------------------------------------
*/

#ifdef TCL_THREADS

static void
defaultSIGCHLDHandler(int sig)
{
int rc;

do {
rc = waitpid(-1, 0, WNOHANG);
} while (rc > 0);

signal(SIGCHLD, &defaultSIGCHLDHandler);
}

#endif

/*
*----------------------------------------------------------------------
*
* Tcl_InitNotifier --
*
* Initializes the platform specific notifier state.
*
* Results:
* Returns a handle to the notifier state for this thread..
*
* Side effects:
* May install a SIGCHLD handler.
*
*----------------------------------------------------------------------
*/

ClientData
Tcl_InitNotifier()
{
ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey);

#ifdef TCL_THREADS
static int signalHandlerInitialized = 0;

Tcl_MutexLock(&notifierMutex);

/*
* Install a default SIGCHLD handler. In the threaded environment,
* signal handlers are shared by all threads, so do this only once.
*/
if (!signalHandlerInitialized) {
void (*oldhandler)(int);

oldhandler = signal(SIGCHLD, &defaultSIGCHLDHandler);

/*
* A handler already exists. We are using the signal only for its
* side effects, so the existing handler (probably set by an
* application embedding Tcl) is good enough for us, so restore it.
*/
if (oldhandler != SIG_DFL) {
signal(SIGCHLD, oldhandler);
}

signalHandlerInitialized = 1;
}

/*
* Initialize the tsd info for our new thread
*/
tsdPtr->running = 1;
tsdPtr->self = Tcl_GetCurrentThread();

numThreads++;

Tcl_MutexUnlock(&notifierMutex);
#endif

return (ClientData) tsdPtr;
}

/*
*----------------------------------------------------------------------
*
* Tcl_FinalizeNotifier --
*
* This function is called to cleanup the notifier state before
* a thread is terminated.
*
* Results:
* None.
*
* Side effects:
* None.
*
*----------------------------------------------------------------------
*/

void
Tcl_FinalizeNotifier(clientData)
ClientData clientData; /* Not used. */
{
#ifdef TCL_THREADS
ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey);

Tcl_MutexLock(&notifierMutex);

/*
* Clean up thread local storage.
*/
ckfree((char *)tsdPtr->pollargs);
tsdPtr->pollargs = NULL;

numThreads--;

Tcl_MutexUnlock(&notifierMutex);
#endif
}

/*
*----------------------------------------------------------------------
*
* Tcl_AlertNotifier --
*
* Used to signal another thread, typically after adding a new
* event to its queue with Tcl_ThreadQueueEvent.
*
* Results:
* None.
*
* Side effects:
* The target thread is unblocked.
*
*----------------------------------------------------------------------
*/

void
Tcl_AlertNotifier(clientData)
ClientData clientData;
{
#ifdef TCL_THREADS
ThreadSpecificData *tsdPtr = (ThreadSpecificData *) clientData;

Tcl_MutexLock(&notifierMutex);

tsdPtr->notified = 1;

/* XXX the following pthread call belongs in the Thread.c file */

if (!tsdPtr->running)
pthread_kill((int)tsdPtr->self, SIGCHLD);

Tcl_MutexUnlock(&notifierMutex);
#endif
}

/*
*----------------------------------------------------------------------
*
* Tcl_SetTimer --
*
* This procedure sets the current notifier timer value. This
* interface is not implemented in this notifier because we are
* always running inside of Tcl_DoOneEvent.
*
* Results:
* None.
*
* Side effects:
* None.
*
*----------------------------------------------------------------------
*/

void
Tcl_SetTimer(timePtr)
Tcl_Time *timePtr; /* Timeout value, may be NULL. */
{
/*
* The interval timer doesn't do anything in this implementation,
* because the only event loop is via Tcl_DoOneEvent, which passes
* timeout values to Tcl_WaitForEvent.
*/
}

/*
*----------------------------------------------------------------------
*
* Tcl_ServiceModeHook --
*
* This function is invoked whenever the service mode changes.
*
* Results:
* None.
*
* Side effects:
* None.
*
*----------------------------------------------------------------------
*/

void
Tcl_ServiceModeHook(mode)
int mode; /* Either TCL_SERVICE_ALL, or
* TCL_SERVICE_NONE. */
{
}

/*
*----------------------------------------------------------------------
*
* Tcl_CreateFileHandler --
*
* This procedure registers a file handler with the select notifier.
*
* Results:
* None.
*
* Side effects:
* Creates a new file handler structure.
*
*----------------------------------------------------------------------
*/

void
Tcl_CreateFileHandler(fd, mask, proc, clientData)
int fd; /* Handle of stream to watch. */
int mask; /* OR'ed combination of TCL_READABLE,
* TCL_WRITABLE, and TCL_EXCEPTION:
* indicates conditions under which
* proc should be called. */
Tcl_FileProc *proc; /* Procedure to call for each
* selected event. */
ClientData clientData; /* Arbitrary data to pass to proc. */
{
ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey);
FileHandler *filePtr;
struct pollfd *pollPtr;
int index;

/*
* Check if we already have a FileHandler struct for this fd
*/
for (filePtr = tsdPtr->firstFileHandlerPtr; filePtr != NULL;
filePtr = filePtr->nextPtr) {
if (filePtr->fd == fd) {
break;
}
}

/*
* Allocate and initialize a new FileHandler
*/
if (filePtr == NULL) {
filePtr = (FileHandler*) ckalloc(sizeof(FileHandler));
filePtr->fd = fd;
filePtr->readyMask = 0;
filePtr->nextPtr = tsdPtr->firstFileHandlerPtr;
tsdPtr->firstFileHandlerPtr = filePtr;
}

/*
* Insert calling args
*/
filePtr->proc = proc;
filePtr->clientData = clientData;
filePtr->mask = mask;

/*
* Allocate a struct pollfd for this file. First check there is room
* in the pollargs array for a new entry.
*/

if (tsdPtr->maxfds == tsdPtr->numfds) {
tsdPtr->maxfds += 4;
tsdPtr->pollargs = (struct pollfd *)ckrealloc((char *)tsdPtr->pollargs,
tsdPtr->maxfds * sizeof(struct pollfd));
}

/*
* Reuse an existing entry
*/
for (index = 0; index < tsdPtr->numfds; index++) {
if (tsdPtr->pollargs[index].fd == fd) break;
}

/*
* If no existing entry, add to end of array
*/
if (index == tsdPtr->numfds) tsdPtr->numfds++;

/*
* Build new pollfd struct for this fd
*/
pollPtr = &tsdPtr->pollargs[index];
pollPtr->fd = fd;
pollPtr->events = 0;
pollPtr->revents = 0;
if (mask & TCL_READABLE) pollPtr->events |= POLLIN;
if (mask & TCL_WRITABLE) pollPtr->events |= POLLOUT;
if (mask & TCL_EXCEPTION) pollPtr->events |= POLLERR;
}

/*
*----------------------------------------------------------------------
*
* Tcl_DeleteFileHandler --
*
* Cancel a previously-arranged callback arrangement for
* a file.
*
* Results:
* None.
*
* Side effects:
* If a callback was previously registered on file, remove it.
*
*----------------------------------------------------------------------
*/

void
Tcl_DeleteFileHandler(fd)
int fd; /* Stream id for which to remove callback procedure. */
{
FileHandler *filePtr, *prevPtr;
int index;
ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey);

/*
* Find the entry for the given file (and return if there isn't one).
*/

for (prevPtr = NULL, filePtr = tsdPtr->firstFileHandlerPtr; ;
prevPtr = filePtr, filePtr = filePtr->nextPtr) {
if (filePtr == NULL) {
return;
}
if (filePtr->fd == fd) {
break;
}
}

/*
* Update the poll arguments for this call
*/

if (tsdPtr->pollargs == NULL) return;

/*
* look up fd in pollargs array
*/
for (index = 0; index < tsdPtr->numfds; index++) {
if (tsdPtr->pollargs[index].fd == fd) break;
}

/*
* free entry in pollargs array by copying last entry over the
* one being deleted (order of entries doesn't matter).
*/
if (index != tsdPtr->numfds && tsdPtr->numfds > 0) {
tsdPtr->pollargs[index] = tsdPtr->pollargs[--tsdPtr->numfds];
}

/*
* Clean up information in the callback record.
*/

if (prevPtr == NULL) {
tsdPtr->firstFileHandlerPtr = filePtr->nextPtr;
} else {
prevPtr->nextPtr = filePtr->nextPtr;
}
ckfree((char *) filePtr);
}

/*
*----------------------------------------------------------------------
*
* FileHandlerEventProc --
*
* This procedure is called by Tcl_ServiceEvent when a file event
* reaches the front of the event queue. This procedure is
* responsible for actually handling the event by invoking the
* callback for the file handler.
*
* Results:
* Returns 1 if the event was handled, meaning it should be removed
* from the queue. Returns 0 if the event was not handled, meaning
* it should stay on the queue. The only time the event isn't
* handled is if the TCL_FILE_EVENTS flag bit isn't set.
*
* Side effects:
* Whatever the file handler's callback procedure does.
*
*----------------------------------------------------------------------
*/

static int
FileHandlerEventProc(evPtr, flags)
Tcl_Event *evPtr; /* Event to service. */
int flags; /* Flags that indicate what events to
* handle, such as TCL_FILE_EVENTS. */
{
int mask;
FileHandler *filePtr;
FileHandlerEvent *fileEvPtr = (FileHandlerEvent *) evPtr;
ThreadSpecificData *tsdPtr;

if (!(flags & TCL_FILE_EVENTS)) {
return 0;
}

/*
* Search through the file handlers to find the one whose handle matches
* the event. We do this rather than keeping a pointer to the file
* handler directly in the event, so that the handler can be deleted
* while the event is queued without leaving a dangling pointer.
*/

tsdPtr = TCL_TSD_INIT(&dataKey);
for (filePtr = tsdPtr->firstFileHandlerPtr; filePtr != NULL;
filePtr = filePtr->nextPtr) {
if (filePtr->fd != fileEvPtr->fd) {
continue;
}

/*
* The code is tricky for two reasons:
* 1. The file handler's desired events could have changed
* since the time when the event was queued, so AND the
* ready mask with the desired mask.
* 2. The file could have been closed and re-opened since
* the time when the event was queued. This is why the
* ready mask is stored in the file handler rather than
* the queued event: it will be zeroed when a new
* file handler is created for the newly opened file.
*/

mask = filePtr->readyMask & filePtr->mask;
filePtr->readyMask = 0;
if (mask != 0) {
(*filePtr->proc)(filePtr->clientData, mask);
}
break;
}
return 1;
}

/*
*----------------------------------------------------------------------
*
* Tcl_WaitForEvent --
*
* This function is called by Tcl_DoOneEvent to wait for new
* events on the message queue. If the block time is 0, then
* Tcl_WaitForEvent just polls without blocking.
*
* Results:
* Returns -1 if the poll would block forever, otherwise
* returns 0.
*
* Side effects:
* Queues file events that are detected by the select.
*
*----------------------------------------------------------------------
*/

int
Tcl_WaitForEvent(timePtr)
Tcl_Time *timePtr; /* Maximum block time, or NULL. */
{
FileHandler *filePtr;
FileHandlerEvent *fileEvPtr;
struct pollfd *pollPtr;
int timeout, index, numFound, mask;
ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey);

/*
* Set up the timeout structure. Note that if there are no events to
* check for, we return with a negative result rather than blocking
* forever.
*/

if (timePtr) {
timeout = timePtr->sec * 1000 + timePtr->usec/1000;
} else {
timeout = -1;
}

#ifdef TCL_THREADS
Tcl_MutexLock(&notifierMutex);
/*
* Don't block if another thread alerted us
*/
if (!tsdPtr->notified) tsdPtr->running = 0;
tsdPtr->notified = 0;
Tcl_MutexUnlock(&notifierMutex);

numFound = 0;
if (!tsdPtr->running) {
#endif
numFound = poll(tsdPtr->pollargs, tsdPtr->numfds, timeout);
#ifdef TCL_THREADS
Tcl_MutexLock(&notifierMutex);
tsdPtr->running = 1;
Tcl_MutexUnlock(&notifierMutex);
}
#endif

/*
* Queue all detected file events before returning.
*/

if (numFound > 0)
for (filePtr = tsdPtr->firstFileHandlerPtr; (filePtr != NULL);
filePtr = filePtr->nextPtr) {

/*
* Find pollfd struct for this fd
*/
for (index = 0; index < tsdPtr->numfds; index++) {
if (tsdPtr->pollargs[index].fd == filePtr->fd) break;
}

/*
* The pollfd does not exist -- this happens when EOF has occurred
*/
if (index == tsdPtr->numfds) continue;

pollPtr = &tsdPtr->pollargs[index];
mask = 0;

/*
* Convert pollfd flags to Tcl flags
*/
if (pollPtr->revents & POLLIN) mask |= TCL_READABLE;
if (pollPtr->revents & POLLOUT) mask |= TCL_WRITABLE;
if (pollPtr->revents & POLLERR) mask |= TCL_EXCEPTION;

/*
* On EOF we must delete the pollfd, lest we loop infinitely on
* this condition
*/
if (pollPtr->revents & POLLHUP) {
tsdPtr->pollargs[index] = tsdPtr->pollargs[--tsdPtr->numfds];
}

if (!mask) {
continue;
}

/*
* Don't bother to queue an event if the mask was previously
* non-zero since an event must still be on the queue.
*/

if (filePtr->readyMask != mask) {
fileEvPtr = (FileHandlerEvent *) ckalloc(sizeof(FileHandlerEvent));
fileEvPtr->header.proc = FileHandlerEventProc;
fileEvPtr->fd = filePtr->fd;
Tcl_QueueEvent((Tcl_Event *) fileEvPtr, TCL_QUEUE_TAIL);
}
filePtr->readyMask |= mask;
}
return 0;
}

[Don Porter]

Logged In: YES
user_id=80530

an attached file would have been nicer,
but thanks. Hopefully the Notifier
maintainer(s) will have a chance to
look it over.

[Daniel A. Steffen]

One immediate concern about this patch is portability of poll() and pthread_kill() to all OSs supported by the current unix notifier. There also appear to be potential races w.r.t the 'running' tsd member (not hard to fix).
It is however difficult to evaluate this properly given that it appears to be based on an very old version of tclUnixNotfy.c.
The inline paste of a whole file here with all whitespace omitted also doesn't make reviewing easier.
Could you please attach a unified diff against tclUnixNotfy.c on HEAD for this?

[Donal K. Fellows]

I wonder whether it is possible to have different notifiers selectable at configure time? Not quite sure how that would work though.

(FWIW, some platforms implement poll() on top of select(), and others do the reverse. There seems to be no way to detect which is really superior. And then there are other non-portable APIs like epoll() which are also possible ways to build a notifier. I'm not keen on doing lots of autogoo to figure out the best, but detecting whether something is buildable at all should be more practical.)