From 4b5747df27b0fd22da3282c8cddb4fa6bde42293 Mon Sep 17 00:00:00 2001 From: Johannes Anderwald Date: Sat, 23 Oct 2010 12:10:56 +0000 Subject: [PATCH] [PORTCLS] - Fix check if the pin can be instantiated another time - Remove hack to close old - Add function FreePin to remove its old reference - Fix memory leaks / reference leaks in WavePci pin implementation - Fix memory / reference leaks in WavePci Close implementation svn path=/trunk/; revision=49238 --- .../backpln/portcls/filter_wavecyclic.cpp | 2 +- .../audio/backpln/portcls/filter_wavepci.cpp | 29 +- .../wdm/audio/backpln/portcls/interfaces.hpp | 10 +- .../audio/backpln/portcls/pin_wavecyclic.cpp | 37 ++- .../wdm/audio/backpln/portcls/pin_wavepci.cpp | 249 +++++++++++++----- .../audio/backpln/portcls/port_wavecyclic.cpp | 6 +- 6 files changed, 244 insertions(+), 89 deletions(-) diff --git a/reactos/drivers/wdm/audio/backpln/portcls/filter_wavecyclic.cpp b/reactos/drivers/wdm/audio/backpln/portcls/filter_wavecyclic.cpp index fe0232d7674..e7882bb8da9 100644 --- a/reactos/drivers/wdm/audio/backpln/portcls/filter_wavecyclic.cpp +++ b/reactos/drivers/wdm/audio/backpln/portcls/filter_wavecyclic.cpp @@ -327,7 +327,7 @@ CPortFilterWaveCyclic::Init( NTSTATUS NTAPI CPortFilterWaveCyclic::FreePin( - IN struct IPortPinWaveCyclic* Pin) + IN PPORTPINWAVECYCLIC Pin) { ULONG Index; diff --git a/reactos/drivers/wdm/audio/backpln/portcls/filter_wavepci.cpp b/reactos/drivers/wdm/audio/backpln/portcls/filter_wavepci.cpp index c62cfbf3d61..639fb86720f 100644 --- a/reactos/drivers/wdm/audio/backpln/portcls/filter_wavepci.cpp +++ b/reactos/drivers/wdm/audio/backpln/portcls/filter_wavepci.cpp @@ -96,13 +96,14 @@ CPortFilterWavePci::NewIrpTarget( return STATUS_UNSUCCESSFUL; } - if (m_Pins[ConnectDetails->PinId] && m_Descriptor->Factory.Instances[ConnectDetails->PinId].CurrentPinInstanceCount) + if (m_Pins[ConnectDetails->PinId] && + (m_Descriptor->Factory.Instances[ConnectDetails->PinId].CurrentPinInstanceCount == m_Descriptor->Factory.Instances[ConnectDetails->PinId].MaxFilterInstanceCount)) { - // release existing instance - PC_ASSERT(0); - m_Pins[ConnectDetails->PinId]->Close(DeviceObject, NULL); + // no available instance + return STATUS_UNSUCCESSFUL; } + // now create the pin Status = NewPortPinWavePci(&Pin); if (!NT_SUCCESS(Status)) @@ -305,6 +306,26 @@ CPortFilterWavePci::Init( return STATUS_SUCCESS; } +NTSTATUS +NTAPI +CPortFilterWavePci::FreePin( + IN struct IPortPinWavePci* Pin) +{ + ULONG Index; + + for(Index = 0; Index < m_Descriptor->Factory.PinDescriptorCount; Index++) + { + if (m_Pins[Index] == Pin) + { + m_Descriptor->Factory.Instances[Index].CurrentPinInstanceCount--; + m_Pins[Index] = NULL; + return STATUS_SUCCESS; + } + } + return STATUS_UNSUCCESSFUL; +} + + NTSTATUS NewPortFilterWavePci( OUT IPortFilterWavePci ** OutFilter) diff --git a/reactos/drivers/wdm/audio/backpln/portcls/interfaces.hpp b/reactos/drivers/wdm/audio/backpln/portcls/interfaces.hpp index dc030ed1168..6caebb951d5 100644 --- a/reactos/drivers/wdm/audio/backpln/portcls/interfaces.hpp +++ b/reactos/drivers/wdm/audio/backpln/portcls/interfaces.hpp @@ -599,6 +599,8 @@ DECLARE_INTERFACE_(IIrpStreamVirtual, IIrpStream) #undef INTERFACE #define INTERFACE IPortFilterWavePci +struct IPortPinWavePci; + DECLARE_INTERFACE_(IPortFilterWavePci, IIrpTarget) { DEFINE_ABSTRACT_UNKNOWN() @@ -607,6 +609,9 @@ DECLARE_INTERFACE_(IPortFilterWavePci, IIrpTarget) STDMETHOD_(NTSTATUS, Init)(THIS_ IN PPORTWAVEPCI Port)PURE; + + STDMETHOD_(NTSTATUS, FreePin)(THIS_ + IN struct IPortPinWavePci* Pin)PURE; }; typedef IPortFilterWavePci *PPORTFILTERWAVEPCI; @@ -614,7 +619,10 @@ typedef IPortFilterWavePci *PPORTFILTERWAVEPCI; #define IMP_IPortFilterPci \ IMP_IIrpTarget; \ STDMETHODIMP_(NTSTATUS) Init(THIS_ \ - IN PPORTWAVEPCI Port) + IN PPORTWAVEPCI Port); \ + STDMETHODIMP_(NTSTATUS) FreePin(THIS_ \ + IN struct IPortPinWavePci* Pin) + /***************************************************************************** * IPortPinWavePci diff --git a/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.cpp b/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.cpp index 003b9b339cd..e615413b7e2 100644 --- a/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.cpp +++ b/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.cpp @@ -960,27 +960,29 @@ CPortPinWaveCyclic::Close( { // free format FreeItem(m_Format, TAG_PORTCLASS); + + // format is freed m_Format = NULL; } if (m_IrpQueue) { - // fixme cancel irps + // cancel remaining irps + m_IrpQueue->CancelBuffers(); + + // release irp queue m_IrpQueue->Release(); - } - - if (m_Port) - { - // release reference to port driver - m_Port->Release(); - m_Port = NULL; + // queue is freed + m_IrpQueue = NULL; } if (m_ServiceGroup) { // remove member from service group m_ServiceGroup->RemoveMember(PSERVICESINK(this)); + + // do not release service group, it is released by the miniport object m_ServiceGroup = NULL; } @@ -999,22 +1001,37 @@ CPortPinWaveCyclic::Close( // set state to stop m_State = KSSTATE_STOP; - DPRINT("Closing stream at Irql %u\n", KeGetCurrentIrql()); + // release stream m_Stream->Release(); + // stream is now freed + m_Stream = NULL; } if (m_Filter) { - // release reference to filter instance + // disconnect pin from filter m_Filter->FreePin((PPORTPINWAVECYCLIC)this); + + // release filter reference m_Filter->Release(); + + // pin is done with filter m_Filter = NULL; } + if (m_Port) + { + // release reference to port driver + m_Port->Release(); + + // work is done for port + m_Port = NULL; + } + Irp->IoStatus.Information = 0; Irp->IoStatus.Status = STATUS_SUCCESS; IoCompleteRequest(Irp, IO_NO_INCREMENT); diff --git a/reactos/drivers/wdm/audio/backpln/portcls/pin_wavepci.cpp b/reactos/drivers/wdm/audio/backpln/portcls/pin_wavepci.cpp index 430ae5e8190..a2b89a4408f 100644 --- a/reactos/drivers/wdm/audio/backpln/portcls/pin_wavepci.cpp +++ b/reactos/drivers/wdm/audio/backpln/portcls/pin_wavepci.cpp @@ -62,8 +62,6 @@ protected: KSAUDIO_POSITION m_Position; ULONG m_StopCount; - ULONG m_Delay; - BOOL m_bUsePrefetch; ULONG m_PrefetchOffset; SUBDEVICE_DESCRIPTOR m_Descriptor; @@ -637,41 +635,85 @@ CPortPinWavePci::Close( IN PDEVICE_OBJECT DeviceObject, IN PIRP Irp) { - ISubdevice *SubDevice; NTSTATUS Status; - PSUBDEVICE_DESCRIPTOR Descriptor; + + if (m_Format) + { + // free format + FreeItem(m_Format, TAG_PORTCLASS); + + // format is freed + m_Format = NULL; + } + + if (m_IrpQueue) + { + // cancel remaining irps + m_IrpQueue->CancelBuffers(); + + // release irp queue + m_IrpQueue->Release(); + + // queue is freed + m_IrpQueue = NULL; + } + if (m_ServiceGroup) { + // remove member from service group m_ServiceGroup->RemoveMember(PSERVICESINK(this)); + + // do not release service group, it is released by the miniport object + m_ServiceGroup = NULL; } if (m_Stream) { if (m_State != KSSTATE_STOP) { - m_Stream->SetState(KSSTATE_STOP); + // stop stream + Status = m_Stream->SetState(KSSTATE_STOP); + if (!NT_SUCCESS(Status)) + { + DPRINT("Warning: failed to stop stream with %x\n", Status); + PC_ASSERT(0); + } } + // set state to stop + m_State = KSSTATE_STOP; + + DPRINT("Closing stream at Irql %u\n", KeGetCurrentIrql()); + + // release stream m_Stream->Release(); + + // stream is now freed + m_Stream = NULL; } - Status = m_Port->QueryInterface(IID_ISubdevice, (PVOID*)&SubDevice); - if (NT_SUCCESS(Status)) + if (m_Filter) { - Status = SubDevice->GetDescriptor(&Descriptor); - if (NT_SUCCESS(Status)) - { - Descriptor->Factory.Instances[m_ConnectDetails->PinId].CurrentPinInstanceCount--; - } - SubDevice->Release(); + // disconnect pin from filter + m_Filter->FreePin((PPORTPINWAVEPCI)this); + + // release filter reference + m_Filter->Release(); + + // pin is done with filter + m_Filter = NULL; } - if (m_Format) + if (m_Port) { - FreeItem(m_Format, TAG_PORTCLASS); - m_Format = NULL; + // release reference to port driver + m_Port->Release(); + + // work is done for port + m_Port = NULL; } + // successfully complete irp Irp->IoStatus.Status = STATUS_SUCCESS; Irp->IoStatus.Information = 0; IoCompleteRequest(Irp, IO_NO_INCREMENT); @@ -756,33 +798,18 @@ CPortPinWavePci::Init( NTSTATUS Status; PKSDATAFORMAT DataFormat; BOOLEAN Capture; + ISubdevice * Subdevice = NULL; + PSUBDEVICE_DESCRIPTOR SubDeviceDescriptor = NULL; - Port->AddRef(); - Filter->AddRef(); - - m_Port = Port; - m_Filter = Filter; - m_KsPinDescriptor = KsPinDescriptor; - m_ConnectDetails = ConnectDetails; - m_Miniport = GetWavePciMiniport(Port); - m_DeviceObject = DeviceObject; - - DataFormat = (PKSDATAFORMAT)(ConnectDetails + 1); - - DPRINT("IPortPinWavePci_fnInit entered\n"); - - m_Format = (PKSDATAFORMAT)AllocateItem(NonPagedPool, DataFormat->FormatSize, TAG_PORTCLASS); - if (!m_Format) - return STATUS_INSUFFICIENT_RESOURCES; - - RtlMoveMemory(m_Format, DataFormat, DataFormat->FormatSize); - + // check if it is a source / sink pin if (KsPinDescriptor->Communication == KSPIN_COMMUNICATION_SINK && KsPinDescriptor->DataFlow == KSPIN_DATAFLOW_IN) { + // sink pin Capture = FALSE; } else if (KsPinDescriptor->Communication == KSPIN_COMMUNICATION_SINK && KsPinDescriptor->DataFlow == KSPIN_DATAFLOW_OUT) { + // source pin Capture = TRUE; } else @@ -792,6 +819,45 @@ CPortPinWavePci::Init( while(TRUE); } + // add port / filter reference + Port->AddRef(); + Filter->AddRef(); + + // initialize pin + m_Port = Port; + m_Filter = Filter; + m_KsPinDescriptor = KsPinDescriptor; + m_ConnectDetails = ConnectDetails; + m_Miniport = GetWavePciMiniport(Port); + m_DeviceObject = DeviceObject; + m_State = KSSTATE_STOP; + m_Capture = Capture; + + DPRINT("IPortPinWavePci_fnInit entered\n"); + + // get dataformat + DataFormat = (PKSDATAFORMAT)(ConnectDetails + 1); + + // allocate data format + m_Format = (PKSDATAFORMAT)AllocateItem(NonPagedPool, DataFormat->FormatSize, TAG_PORTCLASS); + if (!m_Format) + { + // release references + m_Port->Release(); + m_Filter->Release(); + + // no dangling pointers + Port = NULL; + Filter = NULL; + + // failed to allocate data format + return STATUS_INSUFFICIENT_RESOURCES; + } + + // copy data format + RtlMoveMemory(m_Format, DataFormat, DataFormat->FormatSize); + + // allocate new stream Status = m_Miniport->NewStream(&m_Stream, NULL, NonPagedPool, @@ -805,47 +871,81 @@ CPortPinWavePci::Init( DPRINT("IPortPinWavePci_fnInit Status %x\n", Status); if (!NT_SUCCESS(Status)) - return Status; - - if (m_ServiceGroup) { - Status = m_ServiceGroup->AddMember(PSERVICESINK(this)); - if (!NT_SUCCESS(Status)) - { - DPRINT("Failed to add pin to service group\n"); - return Status; - } + // free references + Port->Release(); + Filter->Release(); + + // free data format + FreeItem(m_Format, TAG_PORTCLASS); + + // no dangling pointers + m_Port = NULL; + m_Filter = NULL; + m_Format = NULL; + + // failed to allocate stream + return Status; } - // delay of 10 milisec - m_Delay = Int32x32To64(10, -10000); - + // get allocator requirements for pin Status = m_Stream->GetAllocatorFraming(&m_AllocatorFraming); + if (NT_SUCCESS(Status)) + { + DPRINT("OptionFlags %x RequirementsFlag %x PoolType %x Frames %lu FrameSize %lu FileAlignment %lu\n", + m_AllocatorFraming.OptionsFlags, m_AllocatorFraming.RequirementsFlags, m_AllocatorFraming.PoolType, m_AllocatorFraming.Frames, m_AllocatorFraming.FrameSize, m_AllocatorFraming.FileAlignment); + } + + // allocate new irp queue + Status = NewIrpQueue(&m_IrpQueue); if (!NT_SUCCESS(Status)) { - DPRINT("GetAllocatorFraming failed with %x\n", Status); + // free references + Port->Release(); + Filter->Release(); + m_Stream->Release(); + + // free data format + FreeItem(m_Format, TAG_PORTCLASS); + + // no dangling pointers + m_Port = NULL; + m_Filter = NULL; + m_Format = NULL; + m_Stream = NULL; + + // failed to allocate irp queue + return Status; } - DPRINT("OptionFlags %x RequirementsFlag %x PoolType %x Frames %lu FrameSize %lu FileAlignment %lu\n", - m_AllocatorFraming.OptionsFlags, m_AllocatorFraming.RequirementsFlags, m_AllocatorFraming.PoolType, m_AllocatorFraming.Frames, m_AllocatorFraming.FrameSize, m_AllocatorFraming.FileAlignment); + // initialize irp queue + Status = m_IrpQueue->Init(ConnectDetails, m_AllocatorFraming.FrameSize, m_AllocatorFraming.FileAlignment, NULL); + if (!NT_SUCCESS(Status)) + { + // this should never happen + ASSERT(0); + } - ISubdevice * Subdevice = NULL; // get subdevice interface Status = Port->QueryInterface(IID_ISubdevice, (PVOID*)&Subdevice); if (!NT_SUCCESS(Status)) - return Status; - - PSUBDEVICE_DESCRIPTOR SubDeviceDescriptor = NULL; + { + // this function should never fail + ASSERT(0); + } + // get subdevice descriptor Status = Subdevice->GetDescriptor(&SubDeviceDescriptor); if (!NT_SUCCESS(Status)) { - // failed to get descriptor - Subdevice->Release(); - return Status; + // this function should never fail + ASSERT(0); } + // release subdevice + Subdevice->Release(); + /* set up subdevice descriptor */ RtlZeroMemory(&m_Descriptor, sizeof(SUBDEVICE_DESCRIPTOR)); m_Descriptor.FilterPropertySet = PinWavePciPropertySet; @@ -855,21 +955,30 @@ CPortPinWavePci::Init( m_Descriptor.UnknownMiniport = SubDeviceDescriptor->UnknownMiniport; m_Descriptor.PortPin = (PVOID)this; - - - Status = NewIrpQueue(&m_IrpQueue); - if (!NT_SUCCESS(Status)) - return Status; - - Status = m_IrpQueue->Init(ConnectDetails, m_AllocatorFraming.FrameSize, m_AllocatorFraming.FileAlignment, NULL); - if (!NT_SUCCESS(Status)) + if (m_ServiceGroup) { - DPRINT("IrpQueue_Init failed with %x\n", Status); - return Status; + Status = m_ServiceGroup->AddMember(PSERVICESINK(this)); + if (!NT_SUCCESS(Status)) + { + // free references + m_Stream->Release(); + Port->Release(); + Filter->Release(); + + // free data format + FreeItem(m_Format, TAG_PORTCLASS); + + // no dangling pointers + m_Stream = NULL; + m_Port = NULL; + m_Filter = NULL; + m_Format = NULL; + + // failed to add to service group + return Status; + } } - m_State = KSSTATE_STOP; - m_Capture = Capture; return STATUS_SUCCESS; } diff --git a/reactos/drivers/wdm/audio/backpln/portcls/port_wavecyclic.cpp b/reactos/drivers/wdm/audio/backpln/portcls/port_wavecyclic.cpp index 6566fe67299..41879cd8950 100644 --- a/reactos/drivers/wdm/audio/backpln/portcls/port_wavecyclic.cpp +++ b/reactos/drivers/wdm/audio/backpln/portcls/port_wavecyclic.cpp @@ -275,12 +275,12 @@ CPortWaveCyclic::Init( } // create the subdevice descriptor - Status = PcCreateSubdeviceDescriptor(&m_SubDeviceDescriptor, + Status = PcCreateSubdeviceDescriptor(&m_SubDeviceDescriptor, 4, InterfaceGuids, - 0, + 0, NULL, - 2, + 2, WaveCyclicPropertySet, 0, 0,