[ros-diffs] [cgutman] 52152: [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 listeni...
cgutman at svn.reactos.org
cgutman at svn.reactos.org
Wed Jun 8 22:15:28 UTC 2011
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/listen.c?rev=52152&r1=52151&r2=52152&view=diff
==============================================================================
--- 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/lock.c?rev=52152&r1=52151&r2=52152&view=diff
==============================================================================
--- 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/main.c?rev=52152&r1=52151&r2=52152&view=diff
==============================================================================
--- 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/write.c?rev=52152&r1=52151&r2=52152&view=diff
==============================================================================
--- 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 */
More information about the Ros-diffs
mailing list