[PORTCLS]

- Don't request initializing delayed service request as this is the task of the miniport driver
- Reimplement the service group object:
- Use the initialized timer object when RequestService is called
- Fix possible race conditions when adding / removing a service sink by protecting it with a lock
- Acquire the service group list lock when executing the shared dpc routine

svn path=/trunk/; revision=47197

reactos/drivers/wdm/audio/backpln/portcls/pin_dmus.cpp

@@ -602,7 +602,6 @@ CPortPinDMus::Init(
             DPRINT("Failed to add pin to service group\n");
             return Status;
         }
-        m_ServiceGroup->SupportDelayedService();
     }
 
     Status = m_IrpQueue->Init(ConnectDetails, 0, 0, NULL);

reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.cpp

@@ -1213,7 +1213,6 @@ CPortPinWaveCyclic::Init(
         return Status;
     }
 
-    m_ServiceGroup->SupportDelayedService();
     m_Stream->SetState(KSSTATE_STOP);
     m_State = KSSTATE_STOP;
     m_CommonBufferOffset = 0;
@@ -1224,6 +1223,8 @@ CPortPinWaveCyclic::Init(
     m_Delay = Int32x32To64(10, -10000);
 
     Status = m_Stream->SetNotificationFreq(10, &m_FrameSize);
+    PC_ASSERT(NT_SUCCESS(Status));
+    PC_ASSERT(m_FrameSize);
 
     SilenceBuffer = AllocateItem(NonPagedPool, m_FrameSize, TAG_PORTCLASS);
     if (!SilenceBuffer)

reactos/drivers/wdm/audio/backpln/portcls/pin_wavepci.cpp

@@ -815,7 +815,6 @@ CPortPinWavePci::Init(
             DPRINT("Failed to add pin to service group\n");
             return Status;
         }
-        m_ServiceGroup->SupportDelayedService();
     }
 
     // delay of 10 milisec

reactos/drivers/wdm/audio/backpln/portcls/service_group.cpp

@@ -54,12 +54,10 @@ protected:
 
     LIST_ENTRY m_ServiceSinkHead;
 
-    BOOL m_Initialized;
+    BOOL m_TimerInitialized;
-    BOOL m_TimerActive;
     KTIMER m_Timer;
     KDPC m_Dpc;
-    KEVENT m_Event;
+    KSPIN_LOCK m_Lock;
-    LONG m_ThreadActive;
 
     friend VOID NTAPI IServiceGroupDpc(IN struct _KDPC  *Dpc, IN PVOID  DeferredContext, IN PVOID  SystemArgument1, IN PVOID  SystemArgument2);
 
@@ -105,9 +103,16 @@ CServiceGroup::QueryInterface(
 
 CServiceGroup::CServiceGroup(IUnknown * OuterUnknown)
 {
+    // initialize dpc
     KeInitializeDpc(&m_Dpc, IServiceGroupDpc, (PVOID)this);
+
+    // set highest importance
     KeSetImportanceDpc(&m_Dpc, HighImportance);
-    KeInitializeEvent(&m_Event, NotificationEvent, FALSE);
+
+    // initialize service group list lock
+    KeInitializeSpinLock(&m_Lock);
+
+    // initialize service group list
     InitializeListHead(&m_ServiceSinkHead);
 }
 
@@ -119,15 +124,34 @@ CServiceGroup::RequestService()
 
     DPRINT("CServiceGroup::RequestService() Dpc at Level %u\n", KeGetCurrentIrql());
 
-    if (KeGetCurrentIrql() > DISPATCH_LEVEL)
+    if (m_TimerInitialized)
     {
-        KeInsertQueueDpc(&m_Dpc, NULL, NULL);
+        LARGE_INTEGER DueTime;
-        return;
-    }
 
-    KeRaiseIrql(DISPATCH_LEVEL, &OldIrql);
+        // no due time
-    KeInsertQueueDpc(&m_Dpc, NULL, NULL);
+        DueTime.QuadPart = 0LL;
-    KeLowerIrql(OldIrql);
+
+        // delayed service requested
+        KeSetTimer(&m_Timer, DueTime, &m_Dpc);
+    }
+    else
+    {
+        // check curent irql
+        if (KeGetCurrentIrql() > DISPATCH_LEVEL)
+        {
+            //insert dpc to queue
+            KeInsertQueueDpc(&m_Dpc, NULL, NULL);
+        }
+        else
+        {
+            // raise irql to dispatch level to make dpc fire immediately
+            KeRaiseIrql(DISPATCH_LEVEL, &OldIrql);
+            // insert dpc to queue
+            KeInsertQueueDpc(&m_Dpc, NULL, NULL);
+            // lower irql to old level
+            KeLowerIrql(OldIrql);
+        }
+    }
 }
 
 //---------------------------------------------------------------
@@ -140,18 +164,33 @@ CServiceGroup::AddMember(
     IN PSERVICESINK pServiceSink)
 {
     PGROUP_ENTRY Entry;
+    KIRQL OldLevel;
 
+    // sanity check
     PC_ASSERT_IRQL_EQUAL(PASSIVE_LEVEL);
 
+    // allocate service sink entry
     Entry = (PGROUP_ENTRY)AllocateItem(NonPagedPool, sizeof(GROUP_ENTRY), TAG_PORTCLASS);
     if (!Entry)
+    {
+        // out of memory
         return STATUS_INSUFFICIENT_RESOURCES;
+    }
 
+    // initialize service sink entry
     Entry->pServiceSink = pServiceSink;
+    // increment reference count
     pServiceSink->AddRef();
 
+    // acquire service group list lock
+    KeAcquireSpinLock(&m_Lock, &OldLevel);
+
+    // insert into service sink list
     InsertTailList(&m_ServiceSinkHead, &Entry->Entry);
 
+    // release service group list lock
+    KeReleaseSpinLock(&m_Lock, OldLevel);
+
     return STATUS_SUCCESS;
 }
 
@@ -162,23 +201,45 @@ CServiceGroup::RemoveMember(
 {
     PLIST_ENTRY CurEntry;
     PGROUP_ENTRY Entry;
+    KIRQL OldLevel;
 
+    // sanity check
     PC_ASSERT_IRQL_EQUAL(PASSIVE_LEVEL);
 
+    // acquire service group list lock
+    KeAcquireSpinLock(&m_Lock, &OldLevel);
+
+    // grab first entry
     CurEntry = m_ServiceSinkHead.Flink;
+
+    // loop list until the passed entry is found
     while (CurEntry != &m_ServiceSinkHead)
     {
+        // grab entry
         Entry = CONTAINING_RECORD(CurEntry, GROUP_ENTRY, Entry);
+
+        // check if it matches the passed entry
         if (Entry->pServiceSink == pServiceSink)
         {
+            // remove entry from list
             RemoveEntryList(&Entry->Entry);
+
+            // release service sink reference
             pServiceSink->Release();
+
+            // free service sink entry
             FreeItem(Entry, TAG_PORTCLASS);
-            return;
+
+            // leave loop
+            break;
         }
+        // move to next entry
         CurEntry = CurEntry->Flink;
     }
 
+    // release service group list lock
+    KeReleaseSpinLock(&m_Lock, OldLevel);
+
 }
 
 VOID
@@ -194,73 +255,40 @@ IServiceGroupDpc(
     PGROUP_ENTRY Entry;
     CServiceGroup * This = (CServiceGroup*)DeferredContext;
 
+    // acquire service group list lock
+    KeAcquireSpinLockAtDpcLevel(&This->m_Lock);
+
+    // grab first entry
     CurEntry = This->m_ServiceSinkHead.Flink;
+
+    // loop the list and call the attached service sink/group
     while (CurEntry != &This->m_ServiceSinkHead)
     {
+        //grab current entry
         Entry = (PGROUP_ENTRY)CONTAINING_RECORD(CurEntry, GROUP_ENTRY, Entry);
+
+        // call service sink/group
         Entry->pServiceSink->RequestService();
+
+        // move to next entry
         CurEntry = CurEntry->Flink;
     }
+
+    // release service group list lock
+    KeReleaseSpinLockFromDpcLevel(&This->m_Lock);
 }
 
-
-#if 0
-VOID
-NTAPI
-ServiceGroupThread(IN PVOID StartContext)
-{
-    NTSTATUS Status;
-    KWAIT_BLOCK WaitBlockArray[2];
-    PVOID WaitObjects[2];
-    CServiceGroup * This = (CServiceGroup*)StartContext;
-
-    // Set thread state
-    InterlockedIncrement(&This->m_ThreadActive);
-
-    // Setup the wait objects
-    WaitObjects[0] = &m_Timer;
-    WaitObjects[1] = &m_Event;
-
-    do
-    {
-        // Wait on our objects
-        Status = KeWaitForMultipleObjects(2, WaitObjects, WaitAny, Executive, KernelMode, FALSE, NULL, WaitBlockArray);
-
-        switch(Status)
-        {
-            case STATUS_WAIT_0:
-                IServiceGroupDpc(&This->m_Dpc, (PVOID)This, NULL, NULL);
-                break;
-            case STATUS_WAIT_1:
-                PsTerminateSystemThread(STATUS_SUCCESS);
-                return;
-        }
-    }while(TRUE);
-}
-
-#endif
 VOID
 NTAPI
 CServiceGroup::SupportDelayedService()
 {
-    //NTSTATUS Status;
-    //HANDLE ThreadHandle;
-
     PC_ASSERT_IRQL(DISPATCH_LEVEL);
 
-    if (m_Initialized)
+    // initialize the timer
-        return;
+    KeInitializeTimer(&m_Timer);
 
-    KeInitializeTimerEx(&m_Timer, NotificationTimer);
+    // use the timer to perform service requests
-
+    m_TimerInitialized = TRUE;
-#if 0
-    Status = PsCreateSystemThread(&ThreadHandle, THREAD_ALL_ACCESS, NULL, 0, NULL, ServiceGroupThread, (PVOID)This);
-    if (NT_SUCCESS(Status))
-    {
-        ZwClose(ThreadHandle);
-        m_Initialized = TRUE;
-    }
-#endif
 }
 
 VOID
@@ -270,17 +298,14 @@ CServiceGroup::RequestDelayedService(
 {
     LARGE_INTEGER DueTime;
 
+    // sanity check
     PC_ASSERT_IRQL(DISPATCH_LEVEL);
+    PC_ASSERT(m_TimerInitialized);
 
     DueTime.QuadPart = ullDelay;
 
-    if (m_Initialized)
+    // set the timer
-    {
+    KeSetTimer(&m_Timer, DueTime, &m_Dpc);
-        if (KeGetCurrentIrql() <= DISPATCH_LEVEL)
-            KeSetTimer(&m_Timer, DueTime, &m_Dpc);
-        else
-            KeInsertQueueDpc(&m_Dpc, NULL, NULL);
-    }
 }
 
 VOID
@@ -288,11 +313,10 @@ NTAPI
 CServiceGroup::CancelDelayedService()
 {
     PC_ASSERT_IRQL(DISPATCH_LEVEL);
+    PC_ASSERT(m_TimerInitialized);
 
-    if (m_Initialized)
+    // cancel the timer
-    {
+    KeCancelTimer(&m_Timer);
-        KeCancelTimer(&m_Timer);
-    }
 }
 
 NTSTATUS
@@ -303,19 +327,31 @@ PcNewServiceGroup(
 {
     CServiceGroup * This;
     NTSTATUS Status;
+
     DPRINT("PcNewServiceGroup entered\n");
 
-    This = new(NonPagedPool, TAG_PORTCLASS)CServiceGroup(OuterUnknown);
+    //FIXME support aggregation
-    if (!This)
+    PC_ASSERT(OuterUnknown == NULL);
-        return STATUS_INSUFFICIENT_RESOURCES;
 
+    // allocate a service group object
+    This = new(NonPagedPool, TAG_PORTCLASS)CServiceGroup(OuterUnknown);
+
+    if (!This)
+    {
+        // out of memory
+        return STATUS_INSUFFICIENT_RESOURCES;
+    }
+
+    // request IServiceSink interface
     Status = This->QueryInterface(IID_IServiceSink, (PVOID*)OutServiceGroup);
 
     if (!NT_SUCCESS(Status))
     {
+        // failed to acquire service sink interface
         delete This;
         return Status;
     }
 
+    // done
     return Status;
 }