[janderwald] 40646: - Allocate work item for each filter / pin being opened - Use ExInterlockedInsertTailList to insert a new audio device to prevent race conditions when more than one audio device is present - Free work item in their target worker routine

Author: janderwald
Date: Wed Apr 22 14:57:28 2009
New Revision: 40646

URL: http://svn.reactos.org/svn/reactos?rev=40646&view=rev
Log:
- Allocate work item for each filter / pin being opened
- Use ExInterlockedInsertTailList to insert a new audio device to prevent race conditions when more than one audio device is present
- Free work item in their target worker routine

Modified:
    trunk/reactos/drivers/wdm/audio/sysaudio/control.c
    trunk/reactos/drivers/wdm/audio/sysaudio/deviface.c
    trunk/reactos/drivers/wdm/audio/sysaudio/main.c
    trunk/reactos/drivers/wdm/audio/sysaudio/sysaudio.h

Modified: trunk/reactos/drivers/wdm/audio/sysaudio/control.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/wdm/audio/sysaudio/...
==============================================================================
--- trunk/reactos/drivers/wdm/audio/sysaudio/control.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/wdm/audio/sysaudio/control.c [iso-8859-1] Wed Apr 22 14:57:28 2009
@@ -236,6 +236,8 @@
         {
             SetIrpIoStatus(WorkerContext->Irp, STATUS_UNSUCCESSFUL, 0);
             ExFreePool(WorkerContext->DispatchContext);
+            IoFreeWorkItem(WorkerContext->WorkItem);
+            ExFreePool(WorkerContext);
             return;
         }

@@ -252,6 +254,8 @@
             SetIrpIoStatus(WorkerContext->Irp, STATUS_UNSUCCESSFUL, 0);
             ExFreePool(WorkerContext->DispatchContext);
             ExFreePool(MixerPinConnect);
+            IoFreeWorkItem(WorkerContext->WorkItem);
+            ExFreePool(WorkerContext);
             return;
         }

@@ -391,8 +395,9 @@
     *((PHANDLE)WorkerContext->Irp->UserBuffer) = VirtualPinHandle;

SetIrpIoStatus(WorkerContext->Irp, STATUS_SUCCESS, sizeof(HANDLE));
+    IoFreeWorkItem(WorkerContext->WorkItem);
+    ExFreePool(WorkerContext);
     return;
-

cleanup:
     if (RealFileObject)
@@ -410,7 +415,8 @@

ExFreePool(WorkerContext->DispatchContext);
     SetIrpIoStatus(WorkerContext->Irp, Status, 0);
-
+    IoFreeWorkItem(WorkerContext->WorkItem);
+    ExFreePool(WorkerContext);
 }

NTSTATUS
@@ -722,8 +728,7 @@
     KSPIN_CINSTANCES PinInstances;
     PPIN_WORKER_CONTEXT WorkerContext;
     PDISPATCH_CONTEXT DispatchContext;
-
-
+    PIO_WORKITEM WorkItem;

IoStack = IoGetCurrentIrpStackLocation(Irp);

@@ -806,10 +811,7 @@
         }
     }

-    ASSERT(DeviceExtension->WorkItem);
-    ASSERT(DeviceExtension->WorkerContext);
-
-    /* create worker context */
+    /* create dispatch pin context */
     DispatchContext = ExAllocatePool(NonPagedPool, sizeof(DISPATCH_CONTEXT));
     if (!DispatchContext)
     {
@@ -817,11 +819,24 @@
         return SetIrpIoStatus(Irp, STATUS_NO_MEMORY, 0);
     }

-    // FIXME
-    // mutal exclusion
-
-    /* get worker context */
-    WorkerContext = (PPIN_WORKER_CONTEXT)DeviceExtension->WorkerContext;
+    /* allocate worker context */
+    WorkerContext = ExAllocatePool(NonPagedPool, sizeof(PIN_WORKER_CONTEXT));
+    if (!WorkerContext)
+    {
+        /* no memory */
+        ExFreePool(DispatchContext);
+        return SetIrpIoStatus(Irp, STATUS_NO_MEMORY, 0);
+    }
+
+    /* allocate work item */
+    WorkItem = IoAllocateWorkItem(DeviceObject);
+    if (!WorkerContext)
+    {
+        /* no memory */
+        ExFreePool(DispatchContext);
+        ExFreePool(WorkerContext);
+        return SetIrpIoStatus(Irp, STATUS_NO_MEMORY, 0);
+    }

/* prepare context */
     RtlZeroMemory(WorkerContext, sizeof(PIN_WORKER_CONTEXT));
@@ -841,13 +856,14 @@
     WorkerContext->PinConnect = PinConnect;
     WorkerContext->AudioClient = ClientInfo;
     WorkerContext->DeviceExtension = DeviceExtension;
+    WorkerContext->WorkItem = WorkItem;

DPRINT("Queing Irp %p\n", Irp);
     /* queue the work item */
     IoMarkIrpPending(Irp);
     Irp->IoStatus.Status = STATUS_PENDING;
     Irp->IoStatus.Information = 0;
-    IoQueueWorkItem(DeviceExtension->WorkItem, CreatePinWorkerRoutine, DelayedWorkQueue, (PVOID)WorkerContext);
+    IoQueueWorkItem(WorkItem, CreatePinWorkerRoutine, DelayedWorkQueue, (PVOID)WorkerContext);

/* mark irp as pending */
     return STATUS_PENDING;

Modified: trunk/reactos/drivers/wdm/audio/sysaudio/deviface.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/wdm/audio/sysaudio/...
==============================================================================
--- trunk/reactos/drivers/wdm/audio/sysaudio/deviface.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/wdm/audio/sysaudio/deviface.c [iso-8859-1] Wed Apr 22 14:57:28 2009
@@ -28,7 +28,10 @@
     NTSTATUS Status;
     ULONG BytesReturned;
     PSYSAUDIODEVEXT DeviceExtension;
-    PKSAUDIO_DEVICE_ENTRY DeviceEntry = (PKSAUDIO_DEVICE_ENTRY)Context;
+    PKSAUDIO_DEVICE_ENTRY DeviceEntry;
+    PFILTER_WORKER_CONTEXT Ctx = (PFILTER_WORKER_CONTEXT)Context;
+
+    DeviceEntry = Ctx->DeviceEntry;

DPRINT("Querying filter...\n");
@@ -42,21 +45,13 @@
     if (!NT_SUCCESS(Status))
     {
         DPRINT1("Failed to query number of pins Status %x\n", Status);
-        ObDereferenceObject(DeviceEntry->FileObject);
-        ZwClose(DeviceEntry->Handle);
-        ExFreePool(DeviceEntry->DeviceName.Buffer);
-        ExFreePool(DeviceEntry);
-        return;
+        goto cleanup;
     }

if (!Count)
     {
         DPRINT1("Filter has no pins!\n");
-        ObDereferenceObject(DeviceEntry->FileObject);
-        ZwClose(DeviceEntry->Handle);
-        ExFreePool(DeviceEntry->DeviceName.Buffer);
-        ExFreePool(DeviceEntry);
-        return;
+        goto cleanup;
     }

/* allocate pin array */
@@ -65,11 +60,7 @@
     {
         /* no memory */
         DPRINT1("Failed to allocate memory Block %x\n", Count * sizeof(PIN_INFO));
-        ObDereferenceObject(DeviceEntry->FileObject);
-        ZwClose(DeviceEntry->Handle);
-        ExFreePool(DeviceEntry->DeviceName.Buffer);
-        ExFreePool(DeviceEntry);
-        return;
+        goto cleanup;
     }
     /* clear array */
     RtlZeroMemory(DeviceEntry->Pins, sizeof(PIN_INFO) * Count);
@@ -114,12 +105,27 @@
     }

DPRINT("Num Pins %u Num WaveIn Pins %u Name WaveOut Pins %u\n", DeviceEntry->NumberOfPins, DeviceEntry->NumWaveInPin, DeviceEntry->NumWaveOutPin);
-
+    /* fetch device extension */
     DeviceExtension = (PSYSAUDIODEVEXT)DeviceObject->DeviceExtension;
-
-    InsertTailList(&DeviceExtension->KsAudioDeviceList, &DeviceEntry->Entry);
-    DeviceExtension->NumberOfKsAudioDevices++;
-
+    /* insert new audio device */
+    ExInterlockedInsertTailList(&DeviceExtension->KsAudioDeviceList, &DeviceEntry->Entry, &DeviceExtension->Lock);
+    /* increment audio device count */
+    InterlockedIncrement((PLONG)&DeviceExtension->NumberOfKsAudioDevices);
+
+    /* free work item */
+    IoFreeWorkItem(Ctx->WorkItem);
+    /* free work item context */
+    ExFreePool(Ctx);
+    return;
+
+cleanup:
+
+    ObDereferenceObject(DeviceEntry->FileObject);
+    ZwClose(DeviceEntry->Handle);
+    ExFreePool(DeviceEntry->DeviceName.Buffer);
+    ExFreePool(DeviceEntry);
+    IoFreeWorkItem(Ctx->WorkItem);
+    ExFreePool(Ctx);
 }

NTSTATUS
@@ -177,7 +183,9 @@
     DEVICE_INTERFACE_CHANGE_NOTIFICATION * Event;
     NTSTATUS Status = STATUS_SUCCESS;
     PSYSAUDIODEVEXT DeviceExtension;
-    PKSAUDIO_DEVICE_ENTRY DeviceEntry;
+    PKSAUDIO_DEVICE_ENTRY DeviceEntry = NULL;
+    PIO_WORKITEM WorkItem = NULL;
+    PFILTER_WORKER_CONTEXT Ctx = NULL;
     PDEVICE_OBJECT DeviceObject = (PDEVICE_OBJECT)Context;

DeviceExtension = (PSYSAUDIODEVEXT)DeviceObject->DeviceExtension;
@@ -191,48 +199,67 @@
         DeviceEntry = ExAllocatePool(NonPagedPool, sizeof(KSAUDIO_DEVICE_ENTRY));
         if (!DeviceEntry)
         {
-
+            /* no memory */
             return STATUS_INSUFFICIENT_RESOURCES;
         }

+        /* initialize audio device entry */
         RtlZeroMemory(DeviceEntry, sizeof(KSAUDIO_DEVICE_ENTRY));
+
+        /* allocate filter ctx */
+        Ctx = ExAllocatePool(NonPagedPool, sizeof(FILTER_WORKER_CONTEXT));
+        if (!Ctx)
+        {
+            /* no memory */
+            Status = STATUS_INSUFFICIENT_RESOURCES;
+            goto cleanup;
+        }
+
+        /* allocate work item */
+        WorkItem = IoAllocateWorkItem(DeviceObject);
+        if (!WorkItem)
+        {
+            /* no memory */
+            Status = STATUS_INSUFFICIENT_RESOURCES;
+            goto cleanup;
+        }
+
+        /* set device name */
         DeviceEntry->DeviceName.Length = 0;
         DeviceEntry->DeviceName.MaximumLength = Event->SymbolicLinkName->Length + 5 * sizeof(WCHAR);
         DeviceEntry->DeviceName.Buffer = ExAllocatePool(NonPagedPool, DeviceEntry->DeviceName.MaximumLength);
+
         if (!DeviceEntry->DeviceName.Buffer)
         {
-            ExFreePool(DeviceEntry);
-            return STATUS_INSUFFICIENT_RESOURCES;
+            Status = STATUS_INSUFFICIENT_RESOURCES;
+            goto cleanup;
         }

if (!NT_SUCCESS(RtlAppendUnicodeToString(&DeviceEntry->DeviceName, L"\??\")))
         {
-            ExFreePool(DeviceEntry->DeviceName.Buffer);
-            ExFreePool(DeviceEntry);
-            return STATUS_INSUFFICIENT_RESOURCES;
+            DPRINT1("RtlAppendUnicodeToString failed with %x\n", Status);
+            goto cleanup;
         }

if (!NT_SUCCESS(RtlAppendUnicodeStringToString(&DeviceEntry->DeviceName, Event->SymbolicLinkName)))
         {
-            ExFreePool(DeviceEntry->DeviceName.Buffer);
-            ExFreePool(DeviceEntry);
-            return STATUS_INSUFFICIENT_RESOURCES;
+            DPRINT1("RtlAppendUnicodeStringToString failed with %x\n", Status);
+            goto cleanup;
         }

Status = OpenDevice(&DeviceEntry->DeviceName, &DeviceEntry->Handle, &DeviceEntry->FileObject);
         if (!NT_SUCCESS(Status))
         {
             DPRINT("ZwCreateFile failed with %x\n", Status);
-            ExFreePool(DeviceEntry->DeviceName.Buffer);
-            ExFreePool(DeviceEntry);
-            return Status;
+            goto cleanup;
         }

DPRINT("Successfully opened audio device %u handle %p file object %p device object %p\n", DeviceExtension->NumberOfKsAudioDevices, DeviceEntry->Handle, DeviceEntry->FileObject, DeviceEntry->FileObject->DeviceObject);

-        //FIXME
-        // mutal exclusion
-        IoQueueWorkItem(DeviceExtension->WorkItem, FilterPinWorkerRoutine, DelayedWorkQueue, (PVOID)DeviceEntry);
+        Ctx->DeviceEntry = DeviceEntry;
+        Ctx->WorkItem = WorkItem;
+
+        IoQueueWorkItem(WorkItem, FilterPinWorkerRoutine, DelayedWorkQueue, (PVOID)Ctx);
         return Status;
     }
     else
@@ -241,6 +268,23 @@
         UNIMPLEMENTED
         return STATUS_SUCCESS;
     }
+
+cleanup:
+    if (Ctx)
+        ExFreePool(Ctx);
+
+    if (WorkItem)
+        IoFreeWorkItem(WorkItem);
+
+    if (DeviceEntry)
+    {
+        if (DeviceEntry->DeviceName.Buffer)
+            ExFreePool(DeviceEntry->DeviceName.Buffer);
+
+        ExFreePool(DeviceEntry);
+    }
+
+    return Status;
 }

NTSTATUS

Modified: trunk/reactos/drivers/wdm/audio/sysaudio/main.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/wdm/audio/sysaudio/...
==============================================================================
--- trunk/reactos/drivers/wdm/audio/sysaudio/main.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/wdm/audio/sysaudio/main.c [iso-8859-1] Wed Apr 22 14:57:28 2009
@@ -118,7 +118,7 @@
     RtlZeroMemory(DeviceExtension, sizeof(SYSAUDIODEVEXT));

/* Initialize the mutex */
-    KeInitializeMutex(&DeviceExtension->Mutex, 0);
+    KeInitializeSpinLock(&DeviceExtension->Lock);

/* Initialize the ks audio device list */
     InitializeListHead(&DeviceExtension->KsAudioDeviceList);
@@ -128,22 +128,6 @@
     if (!NT_SUCCESS(Status))
     {
         DPRINT1("KsAllocateDeviceHeader failed with %x\n", Status);
-        goto cleanup;
-    }
-
-    /* allocate work item */
-    DeviceExtension->WorkItem = IoAllocateWorkItem(DeviceObject);
-    if (!DeviceExtension->WorkItem)
-    {
-        DPRINT1("Failed to allocate work item\n");
-        goto cleanup;
-    }
-
-    /* allocate work item context */
-    DeviceExtension->WorkerContext = ExAllocatePool(NonPagedPool, sizeof(PIN_WORKER_CONTEXT));
-    if (!DeviceExtension->WorkerContext)
-    {
-        DPRINT1("Failed to allocate work item context\n");
         goto cleanup;
     }

Modified: trunk/reactos/drivers/wdm/audio/sysaudio/sysaudio.h
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/wdm/audio/sysaudio/...
==============================================================================
--- trunk/reactos/drivers/wdm/audio/sysaudio/sysaudio.h [iso-8859-1] (original)
+++ trunk/reactos/drivers/wdm/audio/sysaudio/sysaudio.h [iso-8859-1] Wed Apr 22 14:57:28 2009
@@ -70,13 +70,10 @@
     LIST_ENTRY KsAudioDeviceList;                       // audio device list
     PVOID KsAudioNotificationEntry;                     // ks audio notification hook
     PVOID EchoCancelNotificationEntry;                  // ks echo cancel notification hook
-    KMUTEX Mutex;                                       // audio device list mutex
+    KSPIN_LOCK Lock;                                    // audio device list mutex

PFILE_OBJECT KMixerFileObject;                      // mixer file object
     HANDLE KMixerHandle;                                // mixer file handle
-
-    PIO_WORKITEM WorkItem;                              // work item for pin creation
-    PVOID WorkerContext;                                // work item context

}SYSAUDIODEVEXT, *PSYSAUDIODEVEXT;

@@ -113,7 +110,15 @@
     PSYSAUDIO_CLIENT AudioClient;
     PSYSAUDIODEVEXT DeviceExtension;
     PKSDATAFORMAT_WAVEFORMATEX MixerFormat;
+    PIO_WORKITEM WorkItem;
 }PIN_WORKER_CONTEXT, *PPIN_WORKER_CONTEXT;
+
+typedef struct
+{
+    PIO_WORKITEM WorkItem;
+    PKSAUDIO_DEVICE_ENTRY DeviceEntry;
+}FILTER_WORKER_CONTEXT, *PFILTER_WORKER_CONTEXT;
+

NTSTATUS
 SysAudioAllocateDeviceHeader(