[AFD]

- Fix a bug in some completion routines that resulted in failures being treated as successes
- Fix various cancellation bugs
- Fixes termination of programs that open listening sockets (Firefox, server.exe, tcpsvcs.exe, telnetd.exe, etc) and should increase stability in these programs as well

svn path=/trunk/; revision=52152

reactos/drivers/network/afd/afd/listen.c

@@ -140,7 +140,13 @@ static NTSTATUS NTAPI ListenComplete
     }
 
     AFD_DbgPrint(MID_TRACE,("Completing listen request.\n"));
-    AFD_DbgPrint(MID_TRACE,("IoStatus was %x\n", FCB->ListenIrp.Iosb.Status));
+    AFD_DbgPrint(MID_TRACE,("IoStatus was %x\n", Irp->IoStatus.Status));
+    
+    if (Irp->IoStatus.Status != STATUS_SUCCESS)
+    {
+        SocketStateUnlock(FCB);
+        return Irp->IoStatus.Status;
+    }
 
     Qelt = ExAllocatePool( NonPagedPool, sizeof(*Qelt) );
     if( !Qelt ) {

reactos/drivers/network/afd/afd/lock.c

@@ -252,10 +252,38 @@ NTSTATUS LostSocket( PIRP Irp ) {
 }
 
 NTSTATUS LeaveIrpUntilLater( PAFD_FCB FCB, PIRP Irp, UINT Function ) {
+    NTSTATUS Status;
+    
+    /* Add the IRP to the queue in all cases (so AfdCancelHandler will work properly) */
     InsertTailList( &FCB->PendingIrpList[Function],
-		    &Irp->Tail.Overlay.ListEntry );
-    IoMarkIrpPending(Irp);
-    (void)IoSetCancelRoutine(Irp, AfdCancelHandler);
+                    &Irp->Tail.Overlay.ListEntry );
+    
+    /* Acquire the cancel spin lock and check the cancel bit */
+    IoAcquireCancelSpinLock(&Irp->CancelIrql);
+    if (!Irp->Cancel)
+    {
+        /* We are not cancelled; we're good to go so
+         * set the cancel routine, release the cancel spin lock,
+         * mark the IRP as pending, and
+         * return STATUS_PENDING to the caller
+         */
+        (void)IoSetCancelRoutine(Irp, AfdCancelHandler);
+        IoReleaseCancelSpinLock(Irp->CancelIrql);
+        IoMarkIrpPending(Irp);
+        Status = STATUS_PENDING;
+    }
+    else
+    {
+        /* We were already cancelled before we were able to register our cancel routine
+         * so we are to call the cancel routine ourselves right here to cancel the IRP
+         * (which handles all the stuff we do above) and return STATUS_CANCELLED to the caller
+         */
+        AfdCancelHandler(IoGetCurrentIrpStackLocation(Irp)->DeviceObject,
+                         Irp);
+        Status = STATUS_CANCELLED;
+    }
+    
     SocketStateUnlock( FCB );
-    return STATUS_PENDING;
+
+    return Status;
 }

reactos/drivers/network/afd/afd/main.c

@@ -762,6 +762,33 @@ AfdDispatch(PDEVICE_OBJECT DeviceObject, PIRP Irp)
     return (Status);
 }
 
+VOID
+CleanupPendingIrp(PIRP Irp, PIO_STACK_LOCATION IrpSp, PAFD_ACTIVE_POLL Poll)
+{
+    PAFD_RECV_INFO RecvReq;
+    PAFD_SEND_INFO SendReq;
+    PAFD_POLL_INFO PollReq;
+    
+    if (IrpSp->Parameters.DeviceIoControl.IoControlCode == IOCTL_AFD_RECV ||
+        IrpSp->MajorFunction == IRP_MJ_READ)
+    {
+        RecvReq = IrpSp->Parameters.DeviceIoControl.Type3InputBuffer;
+        UnlockBuffers(RecvReq->BufferArray, RecvReq->BufferCount, FALSE);
+    }
+    else if (IrpSp->Parameters.DeviceIoControl.IoControlCode == IOCTL_AFD_SEND ||
+             IrpSp->MajorFunction == IRP_MJ_WRITE)
+    {
+        SendReq = IrpSp->Parameters.DeviceIoControl.Type3InputBuffer;
+        UnlockBuffers(SendReq->BufferArray, SendReq->BufferCount, FALSE);
+    }
+    else if (IrpSp->Parameters.DeviceIoControl.IoControlCode == IOCTL_AFD_SELECT)
+    {
+        PollReq = Irp->AssociatedIrp.SystemBuffer;
+        ZeroEvents(PollReq->Handles, PollReq->HandleCount);
+        SignalSocket(Poll, NULL, PollReq, STATUS_CANCELLED);
+    }       
+}
+
 VOID NTAPI
 AfdCancelHandler(PDEVICE_OBJECT DeviceObject,
                  PIRP Irp)
@@ -769,39 +796,46 @@ AfdCancelHandler(PDEVICE_OBJECT DeviceObject,
     PIO_STACK_LOCATION IrpSp = IoGetCurrentIrpStackLocation(Irp);
     PFILE_OBJECT FileObject = IrpSp->FileObject;
     PAFD_FCB FCB = FileObject->FsContext;
-    UINT Function;
-    PAFD_RECV_INFO RecvReq;
-    PAFD_SEND_INFO SendReq;
-    PLIST_ENTRY CurrentEntry;
+    ULONG Function, IoctlCode;
     PIRP CurrentIrp;
+    PLIST_ENTRY CurrentEntry;
     PAFD_DEVICE_EXTENSION DeviceExt = DeviceObject->DeviceExtension;
     KIRQL OldIrql;
     PAFD_ACTIVE_POLL Poll;
-    PAFD_POLL_INFO PollReq;
 
     IoReleaseCancelSpinLock(Irp->CancelIrql);
-
+    
     if (!SocketAcquireStateLock(FCB))
         return;
+    
+    switch (IrpSp->MajorFunction)
+    {
+        case IRP_MJ_DEVICE_CONTROL:
+            IoctlCode = IrpSp->Parameters.DeviceIoControl.IoControlCode;
+            break;
+            
+        case IRP_MJ_READ:
+            IoctlCode = IOCTL_AFD_RECV;
+            break;
+            
+        case IRP_MJ_WRITE:
+            IoctlCode = IOCTL_AFD_SEND;
+            break;
+            
+        default:
+            ASSERT(FALSE);
+            SocketStateUnlock(FCB);
+            return;
+    }
 
-    ASSERT(IrpSp->MajorFunction == IRP_MJ_DEVICE_CONTROL);
-
-    switch (IrpSp->Parameters.DeviceIoControl.IoControlCode)
+    switch (IoctlCode)
     {
         case IOCTL_AFD_RECV:
-        RecvReq = IrpSp->Parameters.DeviceIoControl.Type3InputBuffer;
-	UnlockBuffers(RecvReq->BufferArray, RecvReq->BufferCount, FALSE);
-        /* Fall through */
-
         case IOCTL_AFD_RECV_DATAGRAM:
         Function = FUNCTION_RECV;
         break;
 
         case IOCTL_AFD_SEND:
-        SendReq = IrpSp->Parameters.DeviceIoControl.Type3InputBuffer;
-        UnlockBuffers(SendReq->BufferArray, SendReq->BufferCount, FALSE);
-        /* Fall through */
-
         case IOCTL_AFD_SEND_DATAGRAM:
         Function = FUNCTION_SEND;
         break;
@@ -821,14 +855,13 @@ AfdCancelHandler(PDEVICE_OBJECT DeviceObject,
         while (CurrentEntry != &DeviceExt->Polls)
         {
             Poll = CONTAINING_RECORD(CurrentEntry, AFD_ACTIVE_POLL, ListEntry);
-            CurrentIrp = Poll->Irp;
-            PollReq = CurrentIrp->AssociatedIrp.SystemBuffer;
 
-            if (CurrentIrp == Irp)
+            if (Irp == Poll->Irp)
             {
-                ZeroEvents(PollReq->Handles, PollReq->HandleCount);
-                SignalSocket(Poll, NULL, PollReq, STATUS_CANCELLED);
-                break;
+                CleanupPendingIrp(Irp, IrpSp, Poll);
+                KeReleaseSpinLock(&DeviceExt->Lock, OldIrql);
+                SocketStateUnlock(FCB);
+                return;
             }
             else
             {
@@ -838,8 +871,9 @@ AfdCancelHandler(PDEVICE_OBJECT DeviceObject,
 
         KeReleaseSpinLock(&DeviceExt->Lock, OldIrql);
 
-        /* IRP already completed by SignalSocket */
         SocketStateUnlock(FCB);
+            
+        DbgPrint("WARNING!!! IRP cancellation race could lead to a process hang! (IOCTL_AFD_SELECT)\n");
         return;
             
         default:
@@ -856,7 +890,9 @@ AfdCancelHandler(PDEVICE_OBJECT DeviceObject,
         if (CurrentIrp == Irp)
         {
             RemoveEntryList(CurrentEntry);
-            break;
+            CleanupPendingIrp(Irp, IrpSp, NULL);
+            UnlockAndMaybeComplete(FCB, STATUS_CANCELLED, Irp, 0);
+            return;
         }
         else
         {
@@ -864,7 +900,9 @@ AfdCancelHandler(PDEVICE_OBJECT DeviceObject,
         }
     }
     
-    UnlockAndMaybeComplete(FCB, STATUS_CANCELLED, Irp, 0);
+    SocketStateUnlock(FCB);
+    
+    DbgPrint("WARNING!!! IRP cancellation race could lead to a process hang! (Function: %d)\n", Function);
 }
 
 static VOID NTAPI

reactos/drivers/network/afd/afd/write.c

@@ -185,9 +185,12 @@ static NTSTATUS NTAPI PacketSocketSendComplete
     FCB->SendIrp.InFlightRequest = NULL;
     /* Request is not in flight any longer */
 
-    FCB->PollState |= AFD_EVENT_SEND;
-    FCB->PollStatus[FD_WRITE_BIT] = STATUS_SUCCESS;
-    PollReeval( FCB->DeviceExt, FCB->FileObject );
+    if (Irp->IoStatus.Status == STATUS_SUCCESS)
+    {
+        FCB->PollState |= AFD_EVENT_SEND;
+        FCB->PollStatus[FD_WRITE_BIT] = STATUS_SUCCESS;
+        PollReeval( FCB->DeviceExt, FCB->FileObject );
+    }
 
     if( FCB->State == SOCKET_STATE_CLOSED ) {
         /* Cleanup our IRP queue because the FCB is being destroyed */