[ros-diffs] [arty] 30701: Mostly revert my earlier attempt at fixing IRP cancellation. I'll attack it again later.

Author: arty
Date: Fri Nov 23 21:06:07 2007
New Revision: 30701

URL: http://svn.reactos.org/svn/reactos?rev=30701&view=rev
Log:
Mostly revert my earlier attempt at fixing IRP cancellation.  I'll attack it
again later.

Modified:
    trunk/reactos/drivers/network/afd/afd/lock.c
    trunk/reactos/drivers/network/afd/afd/main.c
    trunk/reactos/drivers/network/afd/afd/read.c
    trunk/reactos/drivers/network/afd/afd/tdiconn.c
    trunk/reactos/drivers/network/afd/afd/write.c
    trunk/reactos/drivers/network/afd/include/afd.h
    trunk/reactos/drivers/network/tcpip/tcpip/dispatch.c
    trunk/reactos/drivers/network/tcpip/tcpip/main.c

Modified: trunk/reactos/drivers/network/afd/afd/lock.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/lo…
==============================================================================
--- trunk/reactos/drivers/network/afd/afd/lock.c (original)
+++ trunk/reactos/drivers/network/afd/afd/lock.c Fri Nov 23 21:06:07 2007
@@ -46,7 +46,7 @@
 			 BOOLEAN Write, BOOLEAN LockAddress ) {
     UINT i;
     /* Copy the buffer array so we don't lose it */
-    UINT Lock = LockAddress ? 2 : 0;
+    UINT Lock = (LockAddress && AddressLen) ? 2 : 0;
     UINT Size = sizeof(AFD_WSABUF) * (Count + Lock);
     PAFD_WSABUF NewBuf = ExAllocatePool( PagedPool, Size * 2 );
     PMDL NewMdl;
@@ -58,16 +58,13 @@
 
         _SEH_TRY {
             RtlCopyMemory( NewBuf, Buf, sizeof(AFD_WSABUF) * Count );
-            if( LockAddress && AddressLen ) {
+            if( LockAddress ) {
                 NewBuf[Count].buf = AddressBuf;
                 NewBuf[Count].len = *AddressLen;
                 Count++;
                 NewBuf[Count].buf = (PVOID)AddressLen;
                 NewBuf[Count].len = sizeof(*AddressLen);
                 Count++;
-            } else if( LockAddress ) {
-                RtlZeroMemory(NewBuf, sizeof(*NewBuf) * 2);
-                Count += 2;
             }
         } _SEH_HANDLE {
             AFD_DbgPrint(MIN_TRACE,("Access violation copying buffer info "

Modified: trunk/reactos/drivers/network/afd/afd/main.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/ma…
==============================================================================
--- trunk/reactos/drivers/network/afd/afd/main.c (original)
+++ trunk/reactos/drivers/network/afd/afd/main.c Fri Nov 23 21:06:07 2007
@@ -152,7 +152,50 @@
 }
 
 VOID DestroySocket( PAFD_FCB FCB ) {
+    UINT i;
+    BOOLEAN ReturnEarly = FALSE;
+    PAFD_IN_FLIGHT_REQUEST InFlightRequest[IN_FLIGHT_REQUESTS];
+
     AFD_DbgPrint(MIN_TRACE,("Called (%x)\n", FCB));
+
+    if( !SocketAcquireStateLock( FCB ) ) return;
+
+    FCB->State = SOCKET_STATE_CLOSED;
+
+    InFlightRequest[0] = &FCB->ListenIrp;
+    InFlightRequest[1] = &FCB->ReceiveIrp;
+    InFlightRequest[2] = &FCB->SendIrp;
+
+    /* Return early here because we might be called in the mean time. */
+    if( FCB->Critical ||
+	FCB->ListenIrp.InFlightRequest ||
+	FCB->ReceiveIrp.InFlightRequest ||
+	FCB->SendIrp.InFlightRequest ) {
+	AFD_DbgPrint(MIN_TRACE,("Leaving socket alive (%x %x %x)\n",
+				FCB->ListenIrp.InFlightRequest,
+				FCB->ReceiveIrp.InFlightRequest,
+				FCB->SendIrp.InFlightRequest));
+        ReturnEarly = TRUE;
+    }
+
+    /* After PoolReeval, this FCB should not be involved in any outstanding
+     * poll requests */
+
+    /* Cancel our pending requests */
+    for( i = 0; i < IN_FLIGHT_REQUESTS; i++ ) {
+	NTSTATUS Status = STATUS_NO_SUCH_FILE;
+	if( InFlightRequest[i]->InFlightRequest ) {
+	    AFD_DbgPrint(MID_TRACE,("Cancelling in flight irp %d (%x)\n",
+				    i, InFlightRequest[i]->InFlightRequest));
+	    InFlightRequest[i]->InFlightRequest->IoStatus.Status = Status;
+	    InFlightRequest[i]->InFlightRequest->IoStatus.Information = 0;
+	    IoCancelIrp( InFlightRequest[i]->InFlightRequest );
+	}
+    }
+
+    SocketStateUnlock( FCB );
+
+    if( ReturnEarly ) return;
 
     if( FCB->Recv.Window )
 	ExFreePool( FCB->Recv.Window );
@@ -175,12 +218,8 @@
 AfdCloseSocket(PDEVICE_OBJECT DeviceObject, PIRP Irp,
 	       PIO_STACK_LOCATION IrpSp)
 {
-    UINT i;
-    AFD_IN_FLIGHT_REQUEST InFlightRequest[IN_FLIGHT_REQUESTS];
     PFILE_OBJECT FileObject = IrpSp->FileObject;
     PAFD_FCB FCB = FileObject->FsContext;
-
-    if (!SocketAcquireStateLock(FCB)) return LostSocket(Irp, FALSE);
 
     AFD_DbgPrint(MID_TRACE,
 		 ("AfdClose(DeviceObject %p Irp %p)\n", DeviceObject, Irp));
@@ -194,56 +233,15 @@
     if( FCB->EventSelect ) ObDereferenceObject( FCB->EventSelect );
 
     FileObject->FsContext = NULL;
-
-    FCB->State = SOCKET_STATE_CLOSED;
-    SocketStateUnlock(FCB);
-
-    InFlightRequest[0] = FCB->ListenIrp;
-    InFlightRequest[1] = FCB->ReceiveIrp;
-    InFlightRequest[2] = FCB->SendIrp;
-
-    /* Return early here because we might be called in the mean time. */
-    if( !(FCB->Critical ||
-          FCB->ListenIrp.InFlightRequest ||
-          FCB->ReceiveIrp.InFlightRequest ||
-          FCB->SendIrp.InFlightRequest) ) {
-	AFD_DbgPrint(MIN_TRACE,("Leaving socket alive (%x %x %x)\n",
-				FCB->ListenIrp.InFlightRequest,
-				FCB->ReceiveIrp.InFlightRequest,
-				FCB->SendIrp.InFlightRequest));
-        SocketStateUnlock(FCB);
-        DestroySocket(FCB);
-        Irp->IoStatus.Status = STATUS_SUCCESS;
-        Irp->IoStatus.Information = 0;
-        IoCompleteRequest(Irp, IO_NO_INCREMENT);
-    }
-    else
-    {
-        /* After PoolReeval, this FCB should not be involved in any outstanding
-         * poll requests */
-        
-        /* Cancel our pending requests */
-        for( i = 0; i < IN_FLIGHT_REQUESTS; i++ ) {
-            NTSTATUS Status = STATUS_NO_SUCH_FILE;
-            if( InFlightRequest[i].InFlightRequest ) {
-                AFD_DbgPrint(MID_TRACE,("Cancelling in flight irp %d (%x)\n",
-                                        i, InFlightRequest[i].InFlightRequest));
-                InFlightRequest[i].InFlightRequest->IoStatus.Status = Status;
-                InFlightRequest[i].InFlightRequest->IoStatus.Information = 0;
-                IoCancelIrp( InFlightRequest[i].InFlightRequest );
-            }
-        }
-
-        FCB->PendingClose = Irp;
-        Irp->IoStatus.Status = STATUS_SUCCESS;
-        Irp->IoStatus.Information = 0;
-        IoCompleteRequest(Irp, IO_NO_INCREMENT);
-    }
-
     DestroySocket( FCB );
+
+    Irp->IoStatus.Status = STATUS_SUCCESS;
+    Irp->IoStatus.Information = 0;
+    IoCompleteRequest(Irp, IO_NO_INCREMENT);
+
     AFD_DbgPrint(MID_TRACE, ("Returning success.\n"));
 
-    return Irp->IoStatus.Status;
+    return STATUS_SUCCESS;
 }
 
 static NTSTATUS STDCALL

Modified: trunk/reactos/drivers/network/afd/afd/read.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/re…
==============================================================================
--- trunk/reactos/drivers/network/afd/afd/read.c (original)
+++ trunk/reactos/drivers/network/afd/afd/read.c Fri Nov 23 21:06:07 2007
@@ -231,6 +231,7 @@
     if( FCB->State == SOCKET_STATE_CLOSED ) {
         AFD_DbgPrint(MIN_TRACE,("!!! CLOSED SOCK GOT A RECEIVE COMPLETE
!!!\n"));
 	SocketStateUnlock( FCB );
+	DestroySocket( FCB );
 	return STATUS_SUCCESS;
     } else if( FCB->State == SOCKET_STATE_LISTENING ) {
         AFD_DbgPrint(MIN_TRACE,("!!! LISTENER GOT A RECEIVE COMPLETE !!!\n"));
@@ -434,12 +435,13 @@
 
     AFD_DbgPrint(MID_TRACE,("Called on %x\n", FCB));
 
+    if( !SocketAcquireStateLock( FCB ) ) return STATUS_UNSUCCESSFUL;
+
     FCB->ReceiveIrp.InFlightRequest = NULL;
-
-    if( !SocketAcquireStateLock( FCB ) ) return STATUS_UNSUCCESSFUL;
 
     if( FCB->State == SOCKET_STATE_CLOSED ) {
 	SocketStateUnlock( FCB );
+	DestroySocket( FCB );
 	return STATUS_SUCCESS;
     }
 

Modified: trunk/reactos/drivers/network/afd/afd/tdiconn.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/td…
==============================================================================
--- trunk/reactos/drivers/network/afd/afd/tdiconn.c (original)
+++ trunk/reactos/drivers/network/afd/afd/tdiconn.c Fri Nov 23 21:06:07 2007
@@ -165,6 +165,54 @@
     return Status;
 }
 
+NTSTATUS
+TdiBuildConnectionInfoPair
+( PTDI_CONNECTION_INFO_PAIR ConnectionInfo,
+  PTRANSPORT_ADDRESS From, PTRANSPORT_ADDRESS To )
+    /*
+     * FUNCTION: Fill a TDI_CONNECTION_INFO_PAIR struct will the two addresses
+     *           given.
+     * ARGUMENTS:
+     *   ConnectionInfo: The pair
+     *   From:           The from address
+     *   To:             The to address
+     * RETURNS:
+     *   Status of the operation
+     */
+{
+    PCHAR LayoutFrame;
+    UINT SizeOfEntry;
+    ULONG TdiAddressSize;
+    PTDI_CONNECTION_INFORMATION FromTdiConn, ToTdiConn;
+
+    /* FIXME: Get from socket information */
+    TdiAddressSize = TdiAddressSizeFromType(From->Address[0].AddressType);
+    SizeOfEntry = TdiAddressSize + sizeof(TDI_CONNECTION_INFORMATION);
+
+    LayoutFrame = (PCHAR)ExAllocatePool(NonPagedPool, 2 * SizeOfEntry);
+
+    if (!LayoutFrame) {
+        AFD_DbgPrint(MIN_TRACE, ("Insufficient resources.\n"));
+        return STATUS_INSUFFICIENT_RESOURCES;
+    }
+
+    RtlZeroMemory( LayoutFrame, 2 * SizeOfEntry );
+
+	FromTdiConn = (PTDI_CONNECTION_INFORMATION)LayoutFrame;
+	ToTdiConn = (PTDI_CONNECTION_INFORMATION)LayoutFrame + SizeOfEntry;
+
+    if (From != NULL) {
+	TdiBuildConnectionInfoInPlace( FromTdiConn, From );
+    } else {
+	TdiBuildNullConnectionInfoInPlace( FromTdiConn,
+					   From->Address[0].AddressType );
+    }
+
+    TdiBuildConnectionInfoInPlace( ToTdiConn, To );
+
+    return STATUS_SUCCESS;
+}
+
 PTA_ADDRESS TdiGetRemoteAddress( PTDI_CONNECTION_INFORMATION TdiConn )
     /*
      * Convenience function that rounds out the abstraction of

Modified: trunk/reactos/drivers/network/afd/afd/write.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/wr…
==============================================================================
--- trunk/reactos/drivers/network/afd/afd/write.c (original)
+++ trunk/reactos/drivers/network/afd/afd/write.c Fri Nov 23 21:06:07 2007
@@ -47,6 +47,7 @@
 
     if( FCB->State == SOCKET_STATE_CLOSED ) {
 	SocketStateUnlock( FCB );
+	DestroySocket( FCB );
 	return STATUS_SUCCESS;
     }
 
@@ -177,6 +178,12 @@
     FCB->SendIrp.InFlightRequest = NULL;
     /* Request is not in flight any longer */
 
+    if( FCB->State == SOCKET_STATE_CLOSED ) {
+	SocketStateUnlock( FCB );
+	DestroySocket( FCB );
+	return STATUS_SUCCESS;
+    }
+
     SocketStateUnlock( FCB );
 
     return STATUS_SUCCESS;

Modified: trunk/reactos/drivers/network/afd/include/afd.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/includ…
==============================================================================
--- trunk/reactos/drivers/network/afd/include/afd.h (original)
+++ trunk/reactos/drivers/network/afd/include/afd.h Fri Nov 23 21:06:07 2007
@@ -147,7 +147,6 @@
     PVOID Context;
     DWORD PollState;
     UINT ContextSize;
-    PIRP PendingClose;
     LIST_ENTRY PendingIrpList[MAX_FUNCTIONS];
     LIST_ENTRY DatagramList;
     LIST_ENTRY PendingConnections;

Modified: trunk/reactos/drivers/network/tcpip/tcpip/dispatch.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/tcpi…
==============================================================================
--- trunk/reactos/drivers/network/tcpip/tcpip/dispatch.c (original)
+++ trunk/reactos/drivers/network/tcpip/tcpip/dispatch.c Fri Nov 23 21:06:07 2007
@@ -53,6 +53,34 @@
 
     return IRPFinish(Irp, STATUS_CANCELLED);
 }
+
+
+VOID DispCancelComplete(
+    PVOID Context)
+/*
+ * FUNCTION: Completes a cancel request
+ * ARGUMENTS:
+ *     Context = Pointer to context information (FILE_OBJECT)
+ */
+{
+    /*KIRQL OldIrql;*/
+    PFILE_OBJECT FileObject;
+    PTRANSPORT_CONTEXT TranContext;
+
+    TI_DbgPrint(DEBUG_IRP, ("Called.\n"));
+
+    FileObject  = (PFILE_OBJECT)Context;
+    TranContext = (PTRANSPORT_CONTEXT)FileObject->FsContext;
+
+    /* Set the cleanup event */
+    KeSetEvent(&TranContext->CleanupEvent, 0, FALSE);
+
+    /* We are expected to release the cancel spin lock */
+    /*IoReleaseCancelSpinLock(OldIrql);*/
+
+    TI_DbgPrint(DEBUG_IRP, ("Leaving.\n"));
+}
+
 
 VOID DispDataRequestComplete(
     PVOID Context,
@@ -128,14 +156,8 @@
     TI_DbgPrint(DEBUG_IRP, ("PostCancel: DoDisconnect done\n"));
 
     DispDataRequestComplete(DisType->Irp, STATUS_CANCELLED, 0);
-}
-
-VOID DispDoPacketCancel( PVOID Data ) {
-    TI_DbgPrint(DEBUG_IRP, ("Called.\n"));
-    PIRP *IrpP = (PIRP *)Data, Irp = *IrpP;
-    Irp->IoStatus.Status = STATUS_CANCELLED;
-    Irp->IoStatus.Information = 0;
-    IoCompleteRequest(Irp, IO_NO_INCREMENT);
+
+    DispCancelComplete(DisType->FileObject);
 }
 
 VOID NTAPI DispCancelRequest(
@@ -154,7 +176,6 @@
     UCHAR MinorFunction;
     DISCONNECT_TYPE DisType;
     PVOID WorkItem;
-    PADDRESS_FILE AddrFile;
     /*NTSTATUS Status = STATUS_SUCCESS;*/
 
     TI_DbgPrint(DEBUG_IRP, ("Called.\n"));
@@ -188,26 +209,26 @@
 	if( !ChewCreate( &WorkItem, sizeof(DISCONNECT_TYPE),
 			 DispDoDisconnect, &DisType ) )
 	    ASSERT(0);
-        return;
+        break;
 
     case TDI_SEND_DATAGRAM:
-        AddrFile = (PADDRESS_FILE)TranContext->Handle.AddressHandle;
 	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;
         }
-        /* Nothing to do.  We don't keep them around. */
+
+        /*DGCancelSendRequest(TranContext->Handle.AddressHandle, Irp);*/
         break;
 
     case TDI_RECEIVE_DATAGRAM:
-        AddrFile = (PADDRESS_FILE)TranContext->Handle.AddressHandle;
 	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;
         }
-        DGRemoveIRP(AddrFile, Irp);
+
+        /*DGCancelReceiveRequest(TranContext->Handle.AddressHandle, Irp);*/
         break;
 
     default:
@@ -215,9 +236,11 @@
         break;
     }
 
+    if( Irp->IoStatus.Status == STATUS_PENDING )
+	IoMarkIrpPending(Irp);
+
     IoReleaseCancelSpinLock(Irp->CancelIrql);
-    IoCompleteRequest(Irp, IO_NO_INCREMENT);
-    
+
     TI_DbgPrint(MAX_TRACE, ("Leaving.\n"));
 }
 
@@ -261,6 +284,9 @@
     IoReleaseCancelSpinLock(Irp->CancelIrql);
 
     DispDataRequestComplete(Irp, STATUS_CANCELLED, 0);
+
+    DispCancelComplete(FileObject);
+
     TI_DbgPrint(MAX_TRACE, ("Leaving.\n"));
 }
 
@@ -796,6 +822,7 @@
   return Status;
 }
 
+
 NTSTATUS DispTdiReceiveDatagram(
     PIRP Irp)
 /*
@@ -812,7 +839,6 @@
   TDI_REQUEST Request;
   NTSTATUS Status;
   ULONG BytesReceived;
-  PADDRESS_FILE AddrFile;
 
   TI_DbgPrint(DEBUG_IRP, ("Called.\n"));
 
@@ -826,8 +852,6 @@
       return STATUS_INVALID_ADDRESS;
     }
 
-  AddrFile = (PADDRESS_FILE)TranContext->Handle.AddressHandle;
-
   /* Initialize a receive request */
   Request.Handle.AddressHandle = TranContext->Handle.AddressHandle;
   Request.RequestNotifyObject  = DispDataRequestComplete;
@@ -847,21 +871,21 @@
 			 &DataBuffer,
 			 &BufferSize );
 
-        Status = DGReceiveDatagram(
-            AddrFile,
-            DgramInfo->ReceiveDatagramInformation,
-            DataBuffer,
-            DgramInfo->ReceiveLength,
-            DgramInfo->ReceiveFlags,
-            DgramInfo->ReturnDatagramInformation,
-            &BytesReceived,
-            (PDATAGRAM_COMPLETION_ROUTINE)DispDataRequestComplete,
-            Irp,
-            Irp);
-        if (Status != STATUS_PENDING) {
-            DispDataRequestComplete(Irp, Status, BytesReceived);
-        } else
-            IoMarkIrpPending(Irp);
+      Status = DGReceiveDatagram(
+	  Request.Handle.AddressHandle,
+	  DgramInfo->ReceiveDatagramInformation,
+	  DataBuffer,
+	  DgramInfo->ReceiveLength,
+	  DgramInfo->ReceiveFlags,
+	  DgramInfo->ReturnDatagramInformation,
+	  &BytesReceived,
+	  (PDATAGRAM_COMPLETION_ROUTINE)DispDataRequestComplete,
+	  Irp,
+          Irp);
+      if (Status != STATUS_PENDING) {
+          DispDataRequestComplete(Irp, Status, BytesReceived);
+      } else
+	  IoMarkIrpPending(Irp);
     }
 
   TI_DbgPrint(DEBUG_IRP, ("Leaving. Status is (0x%X)\n", Status));

Modified: trunk/reactos/drivers/network/tcpip/tcpip/main.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/tcpi…
==============================================================================
--- trunk/reactos/drivers/network/tcpip/tcpip/main.c (original)
+++ trunk/reactos/drivers/network/tcpip/tcpip/main.c Fri Nov 23 21:06:07 2007
@@ -146,6 +146,7 @@
   }
 CP
   Context->CancelIrps = FALSE;
+  KeInitializeEvent(&Context->CleanupEvent, NotificationEvent, FALSE);
 CP
   IrpSp = IoGetCurrentIrpStackLocation(Irp);
   IrpSp->FileObject->FsContext = Context;
@@ -273,11 +274,20 @@
 {
   PIRP Irp;
   PIO_STACK_LOCATION IrpSp;
+  PTRANSPORT_CONTEXT TranContext;
+  KIRQL OldIrql;
 
   Irp         = (PIRP)Context;
   IrpSp       = IoGetCurrentIrpStackLocation(Irp);
+  TranContext = (PTRANSPORT_CONTEXT)IrpSp->FileObject->FsContext;
 
   Irp->IoStatus.Status = Status;
+
+  IoAcquireCancelSpinLock(&OldIrql);
+
+  KeSetEvent(&TranContext->CleanupEvent, 0, FALSE);
+
+  IoReleaseCancelSpinLock(OldIrql);
 }
 
 
@@ -311,6 +321,7 @@
   IoAcquireCancelSpinLock(&OldIrql);
 
   Context->CancelIrps = TRUE;
+  KeResetEvent(&Context->CleanupEvent);
 
   IoReleaseCancelSpinLock(OldIrql);
 
@@ -344,6 +355,12 @@
 
     return STATUS_INVALID_PARAMETER;
   }
+
+  if (Status != STATUS_PENDING)
+    TiCleanupFileObjectComplete(Irp, Status);
+
+  KeWaitForSingleObject(&Context->CleanupEvent,
+    UserRequest, KernelMode, FALSE, NULL);
 
   return Irp->IoStatus.Status;
 }