From 9f345c304d666b0fbda57c30b41a9adf9eeb8787 Mon Sep 17 00:00:00 2001 From: Art Yerkes Date: Tue, 13 Dec 2005 20:17:25 +0000 Subject: [PATCH] Patch to fix bugcheck at exit for various TCP wielding apps. This puts in a work item for socket shutdown that decouples the IRP cancel from the actual (PASSIVE_LEVEL) tcp accounting chores. Uses the CHEW static lib that I put in to gather workitem code into one nice sane happy place. It seems like doing the same to loopback.c is detrimental and I suspect that it's due to nasty reentrancy issues in our code. I'll likely adapt chew lib so we can use it there too. svn path=/trunk/; revision=20149 --- reactos/drivers/lib/ip/transport/tcp/tcp.c | 30 ++++ reactos/drivers/net/tcpip/datalink/lan.c | 152 +++++++------------- reactos/drivers/net/tcpip/include/precomp.h | 2 +- reactos/drivers/net/tcpip/include/tcp.h | 2 + reactos/drivers/net/tcpip/tcpip.xml | 1 + reactos/drivers/net/tcpip/tcpip/dispatch.c | 62 +++++--- reactos/drivers/net/tcpip/tcpip/main.c | 4 + 7 files changed, 134 insertions(+), 119 deletions(-) diff --git a/reactos/drivers/lib/ip/transport/tcp/tcp.c b/reactos/drivers/lib/ip/transport/tcp/tcp.c index fcaa415b42d..324a0fdec5b 100644 --- a/reactos/drivers/lib/ip/transport/tcp/tcp.c +++ b/reactos/drivers/lib/ip/transport/tcp/tcp.c @@ -647,4 +647,34 @@ NTSTATUS TCPGetPeerAddress return STATUS_SUCCESS; } +VOID TCPRemoveIRP( PCONNECTION_ENDPOINT Endpoint, PIRP Irp ) { + PLIST_ENTRY Entry; + PLIST_ENTRY ListHead[4]; + KIRQL OldIrql; + PTDI_BUCKET Bucket; + UINT i = 0; + + ListHead[0] = &Endpoint->ReceiveRequest; + ListHead[1] = &Endpoint->ConnectRequest; + ListHead[2] = &Endpoint->ListenRequest; + ListHead[3] = 0; + + TcpipAcquireSpinLock( &Endpoint->Lock, &OldIrql ); + + for( i = 0; ListHead[i]; i++ ) { + for( Entry = ListHead[i]->Flink; + Entry != ListHead[i]; + Entry = Entry->Flink ) { + Bucket = CONTAINING_RECORD( Entry, TDI_BUCKET, Entry ); + + if( Bucket->Request.RequestContext == Irp ) { + RemoveEntryList( &Bucket->Entry ); + break; + } + } + } + + TcpipReleaseSpinLock( &Endpoint->Lock, OldIrql ); +} + /* EOF */ diff --git a/reactos/drivers/net/tcpip/datalink/lan.c b/reactos/drivers/net/tcpip/datalink/lan.c index b4ba34b53e9..ebbbb211c7f 100644 --- a/reactos/drivers/net/tcpip/datalink/lan.c +++ b/reactos/drivers/net/tcpip/datalink/lan.c @@ -51,11 +51,6 @@ BOOLEAN ProtocolRegistered = FALSE; LIST_ENTRY AdapterListHead; KSPIN_LOCK AdapterListLock; -/* Work around being called back into afd at Dpc level */ -KSPIN_LOCK LanWorkLock; -LIST_ENTRY LanWorkList; -WORK_QUEUE_ITEM LanWorkItem; - /* Double complete protection */ KSPIN_LOCK LanSendCompleteLock; LIST_ENTRY LanSendCompleteList; @@ -289,10 +284,9 @@ VOID STDCALL ProtocolSendComplete( } } -VOID STDCALL LanReceiveWorker( PVOID Context ) { +VOID LanReceiveWorker( PVOID Context ) { UINT PacketType; - PLIST_ENTRY ListEntry; - PLAN_WQ_ITEM WorkItem; + PLAN_WQ_ITEM WorkItem = (PLAN_WQ_ITEM)Context; PNDIS_PACKET Packet; PLAN_ADAPTER Adapter; UINT BytesTransferred; @@ -301,60 +295,49 @@ VOID STDCALL LanReceiveWorker( PVOID Context ) { TI_DbgPrint(DEBUG_DATALINK, ("Called.\n")); - while( (ListEntry = - ExInterlockedRemoveHeadList( &LanWorkList, &LanWorkLock )) ) { - WorkItem = CONTAINING_RECORD(ListEntry, LAN_WQ_ITEM, ListEntry); - - TI_DbgPrint(DEBUG_DATALINK, ("WorkItem: %x\n", WorkItem)); - - Packet = WorkItem->Packet; - Adapter = WorkItem->Adapter; - BytesTransferred = WorkItem->BytesTransferred; - - ExFreePool( WorkItem ); - - IPPacket.NdisPacket = Packet; - - NdisGetFirstBufferFromPacket(Packet, - &NdisBuffer, - &IPPacket.Header, - &IPPacket.ContigSize, - &IPPacket.TotalSize); - - IPPacket.ContigSize = IPPacket.TotalSize = BytesTransferred; - /* Determine which upper layer protocol that should receive - this packet and pass it to the correct receive handler */ - - TI_DbgPrint(MID_TRACE, - ("ContigSize: %d, TotalSize: %d, BytesTransferred: %d\n", - IPPacket.ContigSize, IPPacket.TotalSize, - BytesTransferred)); - - PacketType = PC(IPPacket.NdisPacket)->PacketType; - IPPacket.Position = 0; - - TI_DbgPrint - (DEBUG_DATALINK, - ("Ether Type = %x ContigSize = %d Total = %d\n", - PacketType, IPPacket.ContigSize, IPPacket.TotalSize)); - - switch (PacketType) { - case ETYPE_IPv4: - case ETYPE_IPv6: - TI_DbgPrint(MID_TRACE,("Received IP Packet\n")); - IPReceive(Adapter->Context, &IPPacket); - break; - case ETYPE_ARP: - TI_DbgPrint(MID_TRACE,("Received ARP Packet\n")); - ARPReceive(Adapter->Context, &IPPacket); - default: - break; - } - - FreeNdisPacket( Packet ); + Packet = WorkItem->Packet; + Adapter = WorkItem->Adapter; + BytesTransferred = WorkItem->BytesTransferred; + + IPPacket.NdisPacket = Packet; + + NdisGetFirstBufferFromPacket(Packet, + &NdisBuffer, + &IPPacket.Header, + &IPPacket.ContigSize, + &IPPacket.TotalSize); + + IPPacket.ContigSize = IPPacket.TotalSize = BytesTransferred; + /* Determine which upper layer protocol that should receive + this packet and pass it to the correct receive handler */ + + TI_DbgPrint(MID_TRACE, + ("ContigSize: %d, TotalSize: %d, BytesTransferred: %d\n", + IPPacket.ContigSize, IPPacket.TotalSize, + BytesTransferred)); + + PacketType = PC(IPPacket.NdisPacket)->PacketType; + IPPacket.Position = 0; + + TI_DbgPrint + (DEBUG_DATALINK, + ("Ether Type = %x ContigSize = %d Total = %d\n", + PacketType, IPPacket.ContigSize, IPPacket.TotalSize)); + + switch (PacketType) { + case ETYPE_IPv4: + case ETYPE_IPv6: + TI_DbgPrint(MID_TRACE,("Received IP Packet\n")); + IPReceive(Adapter->Context, &IPPacket); + break; + case ETYPE_ARP: + TI_DbgPrint(MID_TRACE,("Received ARP Packet\n")); + ARPReceive(Adapter->Context, &IPPacket); + default: + break; } - TI_DbgPrint(DEBUG_DATALINK, ("Leaving\n")); - LanReceiveWorkerBusy = FALSE; + + FreeNdisPacket( Packet ); } VOID LanSubmitReceiveWork( @@ -362,34 +345,19 @@ VOID LanSubmitReceiveWork( PNDIS_PACKET Packet, NDIS_STATUS Status, UINT BytesTransferred) { - PLAN_WQ_ITEM WQItem; + LAN_WQ_ITEM WQItem; PLAN_ADAPTER Adapter = (PLAN_ADAPTER)BindingContext; - KIRQL OldIrql; + PVOID LanWorkItem; TI_DbgPrint(DEBUG_DATALINK,("called\n")); - TcpipAcquireSpinLock( &LanWorkLock, &OldIrql ); - - WQItem = ExAllocatePool( NonPagedPool, sizeof(LAN_WQ_ITEM) ); - if( !WQItem ) { - TcpipReleaseSpinLock( &LanWorkLock, OldIrql ); - return; - } + WQItem.Packet = Packet; + WQItem.Adapter = Adapter; + WQItem.BytesTransferred = BytesTransferred; - WQItem->Packet = Packet; - WQItem->Adapter = Adapter; - WQItem->BytesTransferred = BytesTransferred; - InsertTailList( &LanWorkList, &WQItem->ListEntry ); - if( !LanReceiveWorkerBusy ) { - LanReceiveWorkerBusy = TRUE; - ExQueueWorkItem( &LanWorkItem, CriticalWorkQueue ); - TI_DbgPrint(DEBUG_DATALINK, - ("Work item inserted %x %x\n", &LanWorkItem, WQItem)); - } else { - TI_DbgPrint(DEBUG_DATALINK, - ("LAN WORKER BUSY %x %x\n", &LanWorkItem, WQItem)); - } - TcpipReleaseSpinLock( &LanWorkLock, OldIrql ); + if( !ChewCreate + ( &LanWorkItem, sizeof(LAN_WQ_ITEM), LanReceiveWorker, &WQItem ) ) + ASSERT(0); } VOID STDCALL ProtocolTransferDataComplete( @@ -452,7 +420,6 @@ NDIS_STATUS STDCALL ProtocolReceive( PNDIS_PACKET NdisPacket; PLAN_ADAPTER Adapter = (PLAN_ADAPTER)BindingContext; PETH_HEADER EHeader = (PETH_HEADER)HeaderBuffer; - KIRQL OldIrql; TI_DbgPrint(DEBUG_DATALINK, ("Called. (packetsize %d)\n",PacketSize)); @@ -490,12 +457,9 @@ NDIS_STATUS STDCALL ProtocolReceive( TI_DbgPrint(DEBUG_DATALINK, ("Adapter: %x (MTU %d)\n", Adapter, Adapter->MTU)); - TcpipAcquireSpinLock( &LanWorkLock, &OldIrql ); - NdisStatus = AllocatePacketWithBuffer( &NdisPacket, NULL, PacketSize + HeaderBufferSize ); if( NdisStatus != NDIS_STATUS_SUCCESS ) { - TcpipReleaseSpinLock( &LanWorkLock, OldIrql ); return NDIS_STATUS_NOT_ACCEPTED; } @@ -534,7 +498,6 @@ NDIS_STATUS STDCALL ProtocolReceive( BytesTransferred = 0; } } - TcpipReleaseSpinLock( &LanWorkLock, OldIrql ); TI_DbgPrint(DEBUG_DATALINK, ("Calling complete\n")); if (NdisStatus != NDIS_STATUS_PENDING) @@ -1369,10 +1332,8 @@ VOID LANUnregisterProtocol( } VOID LANStartup() { - InitializeListHead( &LanWorkList ); InitializeListHead( &LanSendCompleteList ); KeInitializeSpinLock( &LanSendCompleteLock ); - ExInitializeWorkItem( &LanWorkItem, LanReceiveWorker, NULL ); } VOID LANShutdown() { @@ -1380,15 +1341,6 @@ VOID LANShutdown() { PLAN_WQ_ITEM WorkItem; PLIST_ENTRY ListEntry; - TcpipAcquireSpinLock( &LanWorkLock, &OldIrql ); - while( !IsListEmpty( &LanWorkList ) ) { - ListEntry = RemoveHeadList( &LanWorkList ); - WorkItem = CONTAINING_RECORD(ListEntry, LAN_WQ_ITEM, ListEntry); - FreeNdisPacket( WorkItem->Packet ); - ExFreePool( WorkItem ); - } - TcpipReleaseSpinLock( &LanWorkLock, OldIrql ); - KeAcquireSpinLock( &LanSendCompleteLock, &OldIrql ); while( !IsListEmpty( &LanSendCompleteList ) ) { ListEntry = RemoveHeadList( &LanSendCompleteList ); diff --git a/reactos/drivers/net/tcpip/include/precomp.h b/reactos/drivers/net/tcpip/include/precomp.h index 5e245ed1012..407a76c5ac1 100644 --- a/reactos/drivers/net/tcpip/include/precomp.h +++ b/reactos/drivers/net/tcpip/include/precomp.h @@ -36,4 +36,4 @@ #include #include #include - +#include diff --git a/reactos/drivers/net/tcpip/include/tcp.h b/reactos/drivers/net/tcpip/include/tcp.h index f56937bb2da..b2c2efd62e4 100644 --- a/reactos/drivers/net/tcpip/include/tcp.h +++ b/reactos/drivers/net/tcpip/include/tcp.h @@ -176,4 +176,6 @@ NTSTATUS TCPStartup( NTSTATUS TCPShutdown( VOID); +VOID TCPRemoveIRP( PCONNECTION_ENDPOINT Connection, PIRP Irp ); + #endif /* __TCP_H */ diff --git a/reactos/drivers/net/tcpip/tcpip.xml b/reactos/drivers/net/tcpip/tcpip.xml index 3407cfc83de..99f3e1090dc 100644 --- a/reactos/drivers/net/tcpip/tcpip.xml +++ b/reactos/drivers/net/tcpip/tcpip.xml @@ -9,6 +9,7 @@ oskittcp ndis pseh + chew ntoskrnl hal diff --git a/reactos/drivers/net/tcpip/tcpip/dispatch.c b/reactos/drivers/net/tcpip/tcpip/dispatch.c index 20204f35f81..88b1ab63fe0 100644 --- a/reactos/drivers/net/tcpip/tcpip/dispatch.c +++ b/reactos/drivers/net/tcpip/tcpip/dispatch.c @@ -135,6 +135,30 @@ VOID DispDataRequestComplete( TI_DbgPrint(DEBUG_IRP, ("Done Completing IRP\n")); } +typedef struct _DISCONNECT_TYPE { + UINT Type; + PVOID Context; + PIRP Irp; + PFILE_OBJECT FileObject; +} DISCONNECT_TYPE, *PDISCONNECT_TYPE; + +VOID DispDoDisconnect( PVOID Data ) { + PDISCONNECT_TYPE DisType = (PDISCONNECT_TYPE)Data; + + TI_DbgPrint(DEBUG_IRP, ("PostCancel: DoDisconnect\n")); + TCPDisconnect + ( DisType->Context, + DisType->Type, + NULL, + NULL, + DispDataRequestComplete, + DisType->Irp ); + TI_DbgPrint(DEBUG_IRP, ("PostCancel: DoDisconnect done\n")); + + DispDataRequestComplete(DisType->Irp, STATUS_CANCELLED, 0); + + DispCancelComplete(DisType->FileObject); +} VOID DDKAPI DispCancelRequest( PDEVICE_OBJECT Device, @@ -150,6 +174,8 @@ VOID DDKAPI DispCancelRequest( PTRANSPORT_CONTEXT TranContext; PFILE_OBJECT FileObject; UCHAR MinorFunction; + DISCONNECT_TYPE DisType; + PVOID WorkItem; /*NTSTATUS Status = STATUS_SUCCESS;*/ TI_DbgPrint(DEBUG_IRP, ("Called.\n")); @@ -161,6 +187,8 @@ VOID DDKAPI DispCancelRequest( TI_DbgPrint(DEBUG_IRP, ("IRP at (0x%X) MinorFunction (0x%X) IrpSp (0x%X).\n", Irp, MinorFunction, IrpSp)); + Irp->IoStatus.Status = STATUS_PENDING; + #ifdef DBG if (!Irp->Cancel) TI_DbgPrint(MIN_TRACE, ("Irp->Cancel is FALSE, should be TRUE.\n")); @@ -169,26 +197,22 @@ VOID DDKAPI DispCancelRequest( /* Try canceling the request */ switch(MinorFunction) { case TDI_SEND: - TCPDisconnect - ( TranContext->Handle.ConnectionContext, - TDI_DISCONNECT_RELEASE, - NULL, - NULL, - DispDataRequestComplete, - Irp ); - break; - case TDI_RECEIVE: - TCPDisconnect - ( TranContext->Handle.ConnectionContext, - TDI_DISCONNECT_ABORT | TDI_DISCONNECT_RELEASE, - NULL, - NULL, - DispDataRequestComplete, - Irp ); + DisType.Type = TDI_DISCONNECT_RELEASE | + ((MinorFunction == TDI_RECEIVE) ? TDI_DISCONNECT_ABORT : 0); + DisType.Context = TranContext->Handle.ConnectionContext; + DisType.Irp = Irp; + DisType.FileObject = FileObject; + + TCPRemoveIRP( TranContext->Handle.ConnectionContext, Irp ); + + if( !ChewCreate( &WorkItem, sizeof(DISCONNECT_TYPE), + DispDoDisconnect, &DisType ) ) + ASSERT(0); break; case TDI_SEND_DATAGRAM: + Irp->IoStatus.Status = STATUS_CANCELLED; if (FileObject->FsContext2 != (PVOID)TDI_TRANSPORT_ADDRESS_FILE) { TI_DbgPrint(MIN_TRACE, ("TDI_SEND_DATAGRAM, but no address file.\n")); break; @@ -198,6 +222,7 @@ VOID DDKAPI DispCancelRequest( break; case TDI_RECEIVE_DATAGRAM: + Irp->IoStatus.Status = STATUS_CANCELLED; if (FileObject->FsContext2 != (PVOID)TDI_TRANSPORT_ADDRESS_FILE) { TI_DbgPrint(MIN_TRACE, ("TDI_RECEIVE_DATAGRAM, but no address file.\n")); break; @@ -211,9 +236,10 @@ VOID DDKAPI DispCancelRequest( break; } - IoReleaseCancelSpinLock(Irp->CancelIrql); + if( Irp->IoStatus.Status == STATUS_PENDING ) + IoMarkIrpPending(Irp); - DispCancelComplete(FileObject); + IoReleaseCancelSpinLock(Irp->CancelIrql); TI_DbgPrint(MAX_TRACE, ("Leaving.\n")); } diff --git a/reactos/drivers/net/tcpip/tcpip/main.c b/reactos/drivers/net/tcpip/tcpip/main.c index b6e7c32d50e..9b547b97342 100644 --- a/reactos/drivers/net/tcpip/tcpip/main.c +++ b/reactos/drivers/net/tcpip/tcpip/main.c @@ -619,6 +619,8 @@ VOID STDCALL TiUnload( } TcpipReleaseSpinLock(&AddressFileListLock, OldIrql); #endif + ChewShutdown(); + /* Cancel timer */ KeCancelTimer(&IPTimer); @@ -734,6 +736,8 @@ DriverEntry( return Status; } + ChewInit( IPDeviceObject ); + /* Create RawIP device object */ Status = IoCreateDevice(DriverObject, 0, &strRawDeviceName, FILE_DEVICE_NETWORK, 0, FALSE, &RawIPDeviceObject); -- 2.17.1