RE: [ros-dev] question about some old changes (hartmut)

[Hartmut Birr]

Hi,

>  What was the reason for these changes?   It looks like
> a workaround for 
some other bug.

it is a nice question after more than one year.

> http://cvs.reactos.com/cgi-bin/cvsweb.cgi/reactos/ntoskrnl/ke/wait.c.diff?r
1=1.51&r2=1.52

KeRemoveAllWaitsThread was called in two situations. One was a real
unblocking of the thread, if the thread was waiting on a waitable object
which was signaled. The other was the terminating of the thread by an other
thread. In this case KeRemoveAllWaitsThread has worked if the thread was
waiting on a waitable object. If the thread was simply suspended,
RemoveEntryList has bug checked because Waiter->WaitListEntry wasn't in any
list. The reason for these changes was the implementation of the CTRL-C
handlers in csrss.

- Hartmut

[Gunnar Dalsnes]

>   -----Original Message-----
>  From: ros-dev-bounces(a)reactos.com 
>  [mailto:[e-mail]] On Behalf Of Hartmut Birr
>  Sent: Thursday, November 18, 2004 2:19 AM
>  To: 'ReactOS Development List'
>  Subject: RE: [ros-dev] question about some old changes (hartmut)
>  
>  
>  Hi,
>  
> >  What was the reason for these changes?   It looks
> > like a  
>  workaround for
>  some other bug.
>  
>  it is a nice question after more than one year.
>  
> > http://cvs.reactos.com/cgi-bin/cvsweb.cgi/reactos/ntoskrnl/ke 
>  /wait.c.diff?r
>  1=1.51&r2=1.52
>
>  KeRemoveAllWaitsThread was called in two situations. One was a real
>  unblocking of the thread, if the thread was waiting on a waitable object
>  which was signaled.  
>   The other was the terminating of the thread by an
> other
>  thread. In this case KeRemoveAllWaitsThread has worked if the thread was
>  waiting on a waitable object. If the thread was simply suspended,
>  RemoveEntryList has bug checked because Waiter->WaitListEntry wasn't in 
any
>   list. The reason for these changes was the
> implementation of the CTRL-C
>  handlers in csrss. 
I see that in PsTerminateCurrentThread KeRemoveAllWaitsThread is called.
This is very strange. If the thread is currently running, there shouldn't be
any WaitBlocks linked to the current thread. 

Also, the tests where the list entry is checked for NULL flink/blink and
where the flink/blink is set to NULL when removed is also strange, since it
shouldn't be possible that a threads WaitBlock is not on a DispatcherHeader
list.

Also, you say that if the thread was suspended, the Waiter->WaitListEntry
wasn't in any list. But a suspend is implemented as a wait on an semaphore,
so the the WaitListEntry  should be linked to the semaphore in this case.

I have tried to undo the changes in the commit and removed the call to
KeRemoveAllWaitsThread in PsTerminateCurrentThread, and ros seem to work as
before. Do you remeber what triggered the crash? Terminating a suspended
thread?

Sorry for bugging you with this, but i believe these changes may be hiding
the real bug.

Gunnar


_______________________________________________
Ros-dev mailing list
Ros-dev(a)reactos.com
http://reactos.com:8080/mailman/listinfo/ros-dev

[Hartmut Birr]

>   -----Original Message-----
>  From: ros-dev-bounces(a)reactos.com 
>  [mailto:[e-mail]] On Behalf Of Gunnar Dalsnes
>  Sent: Thursday, November 18, 2004 11:42 PM
>  To: 'ReactOS Development List'
>  Subject: RE: [ros-dev] question about some old changes (hartmut)
>  
>  
>  
>  Also, you say that if the thread was suspended, the 
>  Waiter->WaitListEntry
>  wasn't in any list. But a suspend is implemented as a wait on 
>  an semaphore,
>  so the the WaitListEntry  should be linked to the semaphore 
>  in this case. 
I meant the thread was paused after a timer interrupt or an other thread was
wake up.   

>   
>  I have tried to undo the changes in the commit and removed the call to
>  KeRemoveAllWaitsThread in PsTerminateCurrentThread, and ros 
>  seem to work as
>  before. Do you remeber what triggered the crash? Terminating 
>  a suspended
>  thread?
>   
The crash occurs after hitting a ctrl-c while ros was compiled on ros. The
bugcheck occurs in a sequence RemoveEntryList/KeRemoveAllWaitsThread. I
cannot remember the correct back trace.

- Hartmut