[kaffe] Possibly Bug In the implementation of jcondvar_broadcast function

[Guilhem Lavaux]

dai shaowei wrote:
> Dear Jim Pick:
>     I am currently analysing the source code of Kaffe,I find there are=20
> possibly something wrong in the implemention of the function =20
> "jcondvar_broadcast"and "jthread_stop"  in
> kaffe -1.1.4\kaffe\kaffevm\systems\unix-jhtreads,following is the sourc=
e=20
> code
>=20
> I think the correct implementation should be :
> Your Orginal code: for (condp =3D cv; *condp !=3D 0; condp =3D &(*condp=
)->next)
>             ;
>                  (*condp) =3D lock->waiting;
>                   lock->waiting =3D *condp;
> My Corrected code: for (condp =3D cv; (*condp)->next !=3D 0; condp =3D=20
> &(*condp)->next)
>                   ;
>                  (*condp) ->next =3D lock->waiting;
>                   lock->waiting =3D *cv; after the "for" repeating=20
> clause,*condp =3D=3D 0,this cause the node in the cv can't link the loc=
k=20
> waiting queue,and after the link ,we should set the lock->waiting to be=
=20
> the first element of the new queue,am I right? hoping your reply.
>=20

Hi !

I agree there is a bug here. We are basically doing lock->waiting =3D
lock->waiting which is wrong. However I believe that the loop is right
(though your code may also work). When the loop exits, condp points to
the last (*condp)->next variable (which is NULL by the loop condition.
So we can assign it to lock->waiting by dereferencing condp. However
lock->waiting should be assigned to the first node of the queue, that is
*cv.

I think that when I modified that code I have made an extra copy-paste. ;=
-)

> Your source code:
> if (jtid =3D=3D jthread_current() &&     (jtid->flags &=20
> THREAD_FLAGS_DONTSTOP) !=3D 0 && blockInts =3D=3D 1)
>     die();
> My corrected code:
> if (jtid =3D=3D jthread_current() &&     (jtid->flags &=20
> THREAD_FLAGS_DONTSTOP) =3D=3D 0 && blockInts =3D=3D 1)
>     die();
> because only when we can stop a thread(the flag THREAD_FLAGS_DONTSTOP i=
s=20
> not set) ,we should call the die(),otherwise,we can only resume the thr=
ead
>   I don=A1=AFt know whether my understanding is right, I would be very =
glad=20
> if you tell me your viewpoint.

I agree with that point.

Thank you very much !

Regards,

Guilhem Lavaux.