[kaffe] Possibly Bug In the implementation of jcondvar_broadcast function

dai shaowei shwdai@hotmail.com
Mon May 31 18:06:02 2004


Dear Jim Pick:
     I am currently analysing the source code of Kaffe,I find there are 
possibly something wrong in the implemention of the function  
"jcondvar_broadcast"and "jthread_stop"  in
 kaffe -1.1.4\kaffe\kaffevm\systems\unix-jhtreads,following is the source 
code

void  jcondvar_broadcast(jcondvar *cv, jmutex *lock)
{
    intsDisable();
    if (*cv != NULL) {
    /* splice the lists `*cv' and `lock->waiting' */
    KaffeNodeQueue** condp;
    /* advance to last element in cv list */
    for (condp = cv; *condp != 0; condp = &(*condp)->next)
    ;
    (*condp) = lock->waiting;
     lock->waiting = *condp;
     *condp = NULL;
    }
    intsRestore();
}
I think the correct implementation should be :
Your Orginal code: for (condp = cv; *condp != 0; condp = &(*condp)->next)
	        ;
                  (*condp) = lock->waiting;
                   lock->waiting = *condp;
My Corrected code: for (condp = cv; (*condp)->next != 0; condp = 
&(*condp)->next)
                   ;
                  (*condp) ->next = lock->waiting;
                   lock->waiting = *cv; 
after the "for" repeating clause,*condp == 0,this cause the node in the cv 
can't link the lock waiting queue,and after the link ,we should set the 
lock->waiting to be the first element of the new queue,am I right? hoping 
your reply.

void
jthread_stop(jthread *jtid)
{
     intsDisable();
     /* No reason to hit a dead man over the head */
     if (jtid->status != THREAD_DEAD) {
	jtid->flags |= THREAD_FLAGS_KILLED;
     }

    /* if it's us, die */
    if (jtid == jthread_current() && 
	(jtid->flags & THREAD_FLAGS_DONTSTOP) != 0 && blockInts == 1)
	die();

    resumeThread(jtid);
    IntsRestore();
}
Your source code:
if (jtid == jthread_current() && 
	(jtid->flags & THREAD_FLAGS_DONTSTOP) != 0 && blockInts == 1)
	die();
My corrected code:
if (jtid == jthread_current() && 
	(jtid->flags & THREAD_FLAGS_DONTSTOP) == 0 && blockInts == 1)
	die();
because only when we can stop a thread(the flag THREAD_FLAGS_DONTSTOP is 
not set) ,we should call the die(),otherwise,we can only resume the thread
   I don¡¯t know whether my understanding is right, I would be very glad if 
you tell me your viewpoint.
  Hoping your reply!
                                                                        
steve dai
                                                                            
   2004.5.31