Author: cgutman Date: Wed Jun 8 22:15:27 2011 New Revision: 52152
URL: http://svn.reactos.org/svn/reactos?rev=52152&view=rev Log: [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
Modified: trunk/reactos/drivers/network/afd/afd/listen.c trunk/reactos/drivers/network/afd/afd/lock.c trunk/reactos/drivers/network/afd/afd/main.c trunk/reactos/drivers/network/afd/afd/write.c
Modified: trunk/reactos/drivers/network/afd/afd/listen.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/lis... ============================================================================== --- trunk/reactos/drivers/network/afd/afd/listen.c [iso-8859-1] (original) +++ trunk/reactos/drivers/network/afd/afd/listen.c [iso-8859-1] Wed Jun 8 22:15:27 2011 @@ -140,7 +140,13 @@ }
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 ) {
Modified: trunk/reactos/drivers/network/afd/afd/lock.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/loc... ============================================================================== --- trunk/reactos/drivers/network/afd/afd/lock.c [iso-8859-1] (original) +++ trunk/reactos/drivers/network/afd/afd/lock.c [iso-8859-1] Wed Jun 8 22:15:27 2011 @@ -252,10 +252,38 @@ }
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; +}
Modified: trunk/reactos/drivers/network/afd/afd/main.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/mai... ============================================================================== --- trunk/reactos/drivers/network/afd/afd/main.c [iso-8859-1] (original) +++ trunk/reactos/drivers/network/afd/afd/main.c [iso-8859-1] Wed Jun 8 22:15:27 2011 @@ -762,6 +762,33 @@ 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 @@ 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; + ULONG Function, IoctlCode; + PIRP CurrentIrp; PLIST_ENTRY CurrentEntry; - PIRP CurrentIrp; PAFD_DEVICE_EXTENSION DeviceExt = DeviceObject->DeviceExtension; KIRQL OldIrql; PAFD_ACTIVE_POLL Poll; - PAFD_POLL_INFO PollReq;
IoReleaseCancelSpinLock(Irp->CancelIrql); - + if (!SocketAcquireStateLock(FCB)) return; - - ASSERT(IrpSp->MajorFunction == IRP_MJ_DEVICE_CONTROL); - - switch (IrpSp->Parameters.DeviceIoControl.IoControlCode) + + 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; + } + + 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 @@ 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 @@
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 @@ if (CurrentIrp == Irp) { RemoveEntryList(CurrentEntry); - break; + CleanupPendingIrp(Irp, IrpSp, NULL); + UnlockAndMaybeComplete(FCB, STATUS_CANCELLED, Irp, 0); + return; } else { @@ -864,7 +900,9 @@ } }
- 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
Modified: trunk/reactos/drivers/network/afd/afd/write.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/wri... ============================================================================== --- trunk/reactos/drivers/network/afd/afd/write.c [iso-8859-1] (original) +++ trunk/reactos/drivers/network/afd/afd/write.c [iso-8859-1] Wed Jun 8 22:15:27 2011 @@ -185,9 +185,12 @@ 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 */