[ros-diffs] [cgutman] 47642: [NDIS] - Hold the miniport lock when we work with the timer queue - Use the return value of KeSetTimer(Ex) to determine whether we need to queue the timer in our queue, otherwise we just use the entry that is already there - Add more assertions

[cgutman at svn.reactos.org]

Author: cgutman
Date: Mon Jun  7 00:08:40 2010
New Revision: 47642

URL: http://svn.reactos.org/svn/reactos?rev=47642&view=rev
Log:
[NDIS]
- Hold the miniport lock when we work with the timer queue
- Use the return value of KeSetTimer(Ex) to determine whether we need to queue the timer in our queue, otherwise we just use the entry that is already there
- Add more assertions

Modified:
    trunk/reactos/drivers/network/ndis/ndis/time.c

Modified: trunk/reactos/drivers/network/ndis/ndis/time.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/ndis/ndis/time.c?rev=47642&r1=47641&r2=47642&view=diff
==============================================================================
--- trunk/reactos/drivers/network/ndis/ndis/time.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/network/ndis/ndis/time.c [iso-8859-1] Mon Jun  7 00:08:40 2010
@@ -98,6 +98,8 @@
 BOOLEAN DequeueMiniportTimer(PNDIS_MINIPORT_TIMER Timer)
 {
   PNDIS_MINIPORT_TIMER CurrentTimer;
+
+  ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL);
 
   if (!Timer->Miniport->TimerQueue)
       return FALSE;
@@ -143,13 +145,21 @@
  *     - call at IRQL <= DISPATCH_LEVEL
  */
 {
+  KIRQL OldIrql;
+
   ASSERT_IRQL(DISPATCH_LEVEL);
   ASSERT(TimerCancelled);
   ASSERT(Timer);
 
   *TimerCancelled = KeCancelTimer (&Timer->Timer);
 
-  DequeueMiniportTimer(Timer);
+  if (*TimerCancelled)
+  {
+      KeAcquireSpinLock(&Timer->Miniport->Lock, &OldIrql);
+      /* If it's somebody already dequeued it, something is wrong (maybe a double-cancel?) */
+      if (!DequeueMiniportTimer(Timer)) ASSERT(FALSE);
+      KeReleaseSpinLock(&Timer->Miniport->Lock, OldIrql);
+  }
 }
 
 VOID NTAPI
@@ -166,7 +176,13 @@
                                SystemArgument2);
 
   /* Only dequeue if the timer has a period of 0 */
-  if (!Timer->Timer.Period) DequeueMiniportTimer(Timer);
+  if (!Timer->Timer.Period)
+  {
+      KeAcquireSpinLockAtDpcLevel(&Timer->Miniport->Lock);
+      /* If someone already dequeued it, something is wrong (borked timer implementation?) */
+      if (!DequeueMiniportTimer(Timer)) ASSERT(FALSE);
+      KeReleaseSpinLockFromDpcLevel(&Timer->Miniport->Lock);
+  }
 }
 
 
@@ -224,6 +240,7 @@
  */
 {
   LARGE_INTEGER Timeout;
+  KIRQL OldIrql;
 
   ASSERT_IRQL(DISPATCH_LEVEL);
   ASSERT(Timer);
@@ -231,14 +248,15 @@
   /* relative delays are negative, absolute are positive; resolution is 100ns */
   Timeout.QuadPart = Int32x32To64(MillisecondsPeriod, -10000);
 
-  /* Dequeue the timer if it is queued already */
-  DequeueMiniportTimer(Timer);
-
-  /* Add the timer at the head of the timer queue */
-  Timer->NextDeferredTimer = Timer->Miniport->TimerQueue;
-  Timer->Miniport->TimerQueue = Timer;
-
-  KeSetTimerEx (&Timer->Timer, Timeout, MillisecondsPeriod, &Timer->Dpc);
+  KeAcquireSpinLock(&Timer->Miniport->Lock, &OldIrql);
+  /* If KeSetTimer(Ex) returns FALSE then the timer is not in the system's queue (and not in ours either) */
+  if (!KeSetTimerEx(&Timer->Timer, Timeout, MillisecondsPeriod, &Timer->Dpc))
+  {
+      /* Add the timer at the head of the timer queue */
+      Timer->NextDeferredTimer = Timer->Miniport->TimerQueue;
+      Timer->Miniport->TimerQueue = Timer;
+  }
+  KeReleaseSpinLock(&Timer->Miniport->Lock, OldIrql);
 }
 
 
@@ -262,6 +280,7 @@
  */
 {
   LARGE_INTEGER Timeout;
+  KIRQL OldIrql;
 
   ASSERT_IRQL(DISPATCH_LEVEL);
   ASSERT(Timer);
@@ -269,14 +288,15 @@
   /* relative delays are negative, absolute are positive; resolution is 100ns */
   Timeout.QuadPart = Int32x32To64(MillisecondsToDelay, -10000);
 
-  /* Dequeue the timer if it is queued already */
-  DequeueMiniportTimer(Timer);
-
-  /* Add the timer at the head of the timer queue */
-  Timer->NextDeferredTimer = Timer->Miniport->TimerQueue;
-  Timer->Miniport->TimerQueue = Timer;
-
-  KeSetTimer (&Timer->Timer, Timeout, &Timer->Dpc);
+  KeAcquireSpinLock(&Timer->Miniport->Lock, &OldIrql);
+  /* If KeSetTimer(Ex) returns FALSE then the timer is not in the system's queue (and not in ours either) */
+  if (!KeSetTimer(&Timer->Timer, Timeout, &Timer->Dpc))
+  {
+      /* Add the timer at the head of the timer queue */
+      Timer->NextDeferredTimer = Timer->Miniport->TimerQueue;
+      Timer->Miniport->TimerQueue = Timer;
+  }
+  KeReleaseSpinLock(&Timer->Miniport->Lock, OldIrql);
 }