Re: [ros-diffs] [ion] 19352: - Reimplement Fast Mutex implementation in HAL/NT to be compatible with the real implementation. (Fast Mutex needs to raise IRQL).

[Hartmut Birr]

[e-mail] wrote:
> Reimplement Fast Mutex implementation in HAL/NT to be compatible with the real implementation. (Fast Mutex needs to raise IRQL).
> Implement ExEnterCriticalRegionAndAcquireFastMutexUnsafe and ExReleaseFastMutexUnsafeAndLeaveCriticalRegion.
> Make win32k use those two new functions so that it can continue running at PASSIVE_LEVEL.
> Remove CcBrokenMutex and use the new APIs instead.
> Implement and export ntoskrnl version of Fast Mutex
> Update headers for new fast-mutex definition and API exports.
> Fix RemoveEntryList in NDK.
> Add exfuncs.h to NDK.
> Fix path in mmtypes.h in NDK to be compatible to how it shoudl be included.
>
> *Modified: trunk/reactos/hal/halx86/generic/fmutex.c*
> --- trunk/reactos/hal/halx86/generic/fmutex.c	2005-11-19 21:07:25 UTC (rev 19351)
> +++ trunk/reactos/hal/halx86/generic/fmutex.c	2005-11-19 22:13:35 UTC (rev 19352)
> +VOID
> +FASTCALL
> +ExAcquireFastMutex(PFAST_MUTEX FastMutex)
>
> {
> KeEnterCriticalRegion();
> ExAcquireFastMutexUnsafe(FastMutex);
> KIRQL OldIrql;
> /* Raise IRQL to APC */
> OldIrql = KfRaiseIrql(APC_LEVEL);
> /* Decrease the count */
> if (InterlockedDecrement(&FastMutex->Count))
> {
>    /* Someone is still holding it, use slow path */
>    FastMutex->Contention++;
>    KeWaitForSingleObject(&FastMutex->Gate,
>                          WrExecutive,
>                          WaitAny,
>                          FALSE,
>                          NULL);
> }
> /* Set the owner and IRQL */
> FastMutex->Owner = KeGetCurrentThread();
> FastMutex->OldIrql = OldIrql;
>
> }

Hi,

this piece of  code is wrong. If we terminating a thread, which waits on
something, we do unblock the thread. The thread must check, if it got
the lock, if not it must wait again.

- Hartmut

[Alex Ionescu]

Hartmut Birr wrote:
> Hi,
>
> this piece of  code is wrong. If we terminating a thread, which waits on
> something, we do unblock the thread. The thread must check, if it got
> the lock, if not it must wait again.
> Hartmut

Hi,

I'm afraid that's incorrect; if the wait gets satisfied you -will- get 
the fast mutex.

Best regards,
Alex Ionescu

[Alex Ionescu]

Hartmut Birr wrote:
> Hi,
>
> this piece of  code is wrong. If we terminating a thread, which waits on
> something, we do unblock the thread. The thread must check, if it got
> the lock, if not it must wait again.
> Hartmut

Hi,

Forgot to add in my previous email. You can't terminate a thread that's 
running at APC_LEVEL because the kill APC won't execute. Hence, it's 
impossible for the scenario you mentionned to occur.

Best regards,
Alex Ionescu

[Hartmut Birr]

Alex Ionescu wrote:
> Hartmut Birr wrote:
> > Hi,
> >
> > this piece of  code is wrong. If we terminating a thread, which waits on
> > something, we do unblock the thread. The thread must check, if it got
> > the lock, if not it must wait again.
> > Hartmut
>
> Hi,
>
> Forgot to add in my previous email. You can't terminate a thread
> that's running at APC_LEVEL because the kill APC won't execute. Hence,
> it's impossible for the scenario you mentionned to occur.
>
> Best regards,
> Alex Ionescu

The thread does wait at passive level. It has called
ExEnterCriticalRegionAndAcquireFastMutexUnsafe. A other thread tries to
terminate the waiting thread. KiInsertQueueApc does unwait the waiting
thread, because the exit APC is a special APC.

- Hartmut

[Alex Ionescu]

Hartmut Birr wrote:
> The thread does wait at passive level. It has called
> ExEnterCriticalRegionAndAcquireFastMutexUnsafe. A other thread tries to
> terminate the waiting thread. KiInsertQueueApc does unwait the waiting
> thread, because the exit APC is a special APC.
> Hartmut

Hi,

The exit APC is a normal APC. Here is a very document on MSDN you can 
read, it answered a lot of my questions about locking, APCs and threads:
http://msdn.microsoft.com/library/en-us/dndevice/html/Locks_Sync.asp

Best regards,
Alex Ionescu