Bug in pth_sched_eventmanager() -- can't yield to a thread waiting on an fd

The good news or the bad news first? The bad news you say? Well...

The bad news is that I've just recently been bitten by what I believe to be a 
bug in pth_sched_eventmanager().

The good news is that I think I've fixed it!

Ok, some more details. I'm using NGPT 2.0.1 on a more or less stock SuSE 7.2 
installation, but with an appropriately patched 2.4.19 kernel.

The problem relates to the way that the scheduler selects a new thread to run.

At the end of the scheduler main loop, at around line 700 of pth_sched.c, 
pth_scheduler() calls pth_sched_eventmanager() in order to process any events 
which have occurred and to move those threads which are waiting on those 
threads from the WAITING queue to the READY queue. If the READY queue is not 
empty, then pth_sched_eventmanager() is called with a flag which indicates 
that it should only poll for any events which may have occurred, rather than 
waiting for one to occur. So far so good.

The problem lies in the way that pth_sched_eventmanager() uses that "dopoll" 
flag. Ultimately, what it's used for is to decide whether or not to call 
select() with a zero or a non-zero timeout. Presumably if we already have 
threads in the READY queue, we want to poll (ie call select() with a zero 
timeout) so that if nothing turns up from the select() call we can then go 
straight on and schedule a thread that is already in the READY queue. However 
at several points in pth_sched_eventmanager() the "dopoll" flag is tested and 
used to short-circuit parts of the event checking altogether. This I think is 
the heart of the problem. It means that if we have threads which are 
currently on the READY queue, then some events will not be checked for, 
potentially resulting in starvation problems.

The circumstances in which I experienced this problem involved a CPU bound 
thread (call it thread A) which would occasionally call pthread_yield_np() to 
allow other threads to run, and a thread performing a recv() from a socket 
(call it thread B). Despite the socket being ready to read (ie data was 
arriving from the network), and thread A yielding quite regularly, thread B 
would not return from the recv() call. Eventually I tracked this down to the 
problem in the scheduler. When thread A yields, it goes back on the READY 
queue, so when it comes time for the scheduler to service events it sets the 
"dopoll" flag since the READY queue is non-empty. However the "dopoll" flag 
was causing checking of file descriptor related events to be short-circuited, 
so the socket event which should have woken thread B was never being checked. 
Thus the scheduler just ran thread A again. And again. And again.

I have a simple test case to illustrate this problem. This program creates a 
CPU bound thread (in this case just a tight loop) which regularly yields, and 
a thread which reads from stdin and in reponse writes stuff to stdout. With 
the stock standard NGPT 2.0.1, the thread which reads from stdin never seems 
to run, as no output is ever generated.

==================================================================

#include <pthread.h>


void *cpu_bound_thread_entry(void *arg)
{
    while (1) pthread_yield_np();
}


void *stdin_reading_thread_entry(void *arg)
{
    while (1)
    {
        ssize_t n;
        const size_t bufLen = 20;
        char buf[bufLen];
        n = read(0, buf, bufLen);
        if (n > 0)
        {
            write(1, "Read some data ==> ", 19);
            write(1, buf, n);
            write(1, "\n", 1);
        }
        else if (n == 0)
        {
            write(1, "*EOF*\n", 6);
            break;
        }
        else
        {
            write(1, "*ERROR*\n", 8);
            break;
        }
    }
}


int main(int argc, char *argv[])
{
    pthread_t cpu_bound_thread;
    pthread_t stdin_reading_thread;

    pthread_create(&stdin_reading_thread, 0, stdin_reading_thread_entry, 0);
    pthread_create(&cpu_bound_thread, 0, cpu_bound_thread_entry, 0);

    pthread_join(stdin_reading_thread, 0);
    pthread_join(cpu_bound_thread, 0);
}

==================================================================

However if the NGPT library is patched to remove the short-circuiting of the 
event tests based on "dopoll" this program echos the data read as expected. 
The patch I applied is as follows...

==================================================================

--- ngpt-2.0.1/pth_sched.c      Sat Aug 10 05:31:32 2002
+++ ngpt-2.0.1-hacks/pth_sched.c        Thu Aug 15 18:17:32 2002
@@ -840,7 +840,7 @@
                    this_occurred = FALSE;
 
                    /* Filedescriptor I/O */
-                   if (ev->ev_type == PTH_EVENT_FD && !dopoll && (ds == 
descr)) {
+                   if (ev->ev_type == PTH_EVENT_FD) {
                        /* filedescriptors are checked later all at once.
                           Here we only assemble them in the fd sets */
                        if (ev->ev_goal & PTH_UNTIL_FD_READABLE)
@@ -1034,10 +1034,6 @@
     /* let select() wait for the read-part of the pipe */
     FD_SET(descr->sigpipe[0], &rfds);
 
-    /* If dopoll is set, handle the thread from ready queue... */
-    if (dopoll)
-       return;
-
     if (fdmax < descr->sigpipe[0])
         fdmax = descr->sigpipe[0];

==================================================================

I doubt that this patch is the last word in solving this problem, largely 
because I've got no idea as to the details of how the M:N threading model is 
handled, and how all of that relates to bound threads etc etc. In particular, 
in the first change in the patch I've removed the (ds == dsecr) part of the 
expression too, but I've no idea if this is correct or not. Also, I've been 
working solely on a uniprocessor machine, so I've not been able to verify its 
correct operation in an SMP environment.

However, it does get the test program (and my actual software project!) 
running correctly, so I'm sure it is a start!

Regards
M.Beach