From c5a8c1281c0d645475c85d07e13e41178dbba6a0 Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Tue, 24 Mar 2009 02:01:46 +0000 Subject: [PATCH] - Fix several cancellation and socket shutdown issues: - Don't call DestroySocket if FCB->State == SOCKET_STATE_CLOSED because the FCB is already being destroyed (Irp->Cancel checks hid this bug) - Remove the Irp->Cancel checks (hacks) - Only return early if we can't cancel an IRP - Add an FCB->State check in StreamSocketConnectComplete - Store the failure status in the IRP svn path=/trunk/; revision=40200 --- reactos/drivers/network/afd/afd/connect.c | 22 +++++++++++++++++---- reactos/drivers/network/afd/afd/listen.c | 8 -------- reactos/drivers/network/afd/afd/main.c | 24 ++++------------------- reactos/drivers/network/afd/afd/read.c | 16 --------------- reactos/drivers/network/afd/afd/write.c | 16 --------------- 5 files changed, 22 insertions(+), 64 deletions(-) diff --git a/reactos/drivers/network/afd/afd/connect.c b/reactos/drivers/network/afd/afd/connect.c index 27b759b0ef5..98404f86851 100644 --- a/reactos/drivers/network/afd/afd/connect.c +++ b/reactos/drivers/network/afd/afd/connect.c @@ -84,16 +84,22 @@ static NTSTATUS NTAPI StreamSocketConnectComplete /* I was wrong about this before as we can have pending writes to a not * yet connected socket */ - if( !SocketAcquireStateLock( FCB ) ) return STATUS_FILE_CLOSED; + if( !SocketAcquireStateLock( FCB ) ) { + Irp->IoStatus.Status = STATUS_FILE_CLOSED; + Irp->IoStatus.Information = 0; + return STATUS_FILE_CLOSED; + } AFD_DbgPrint(MID_TRACE,("Irp->IoStatus.Status = %x\n", Irp->IoStatus.Status)); FCB->ConnectIrp.InFlightRequest = NULL; - if( Irp->Cancel ) { - SocketStateUnlock( FCB ); - return STATUS_CANCELLED; + if( FCB->State == SOCKET_STATE_CLOSED ) { + Irp->IoStatus.Status = STATUS_FILE_CLOSED; + Irp->IoStatus.Information = 0; + SocketStateUnlock( FCB ); + return STATUS_FILE_CLOSED; } if( NT_SUCCESS(Irp->IoStatus.Status) ) { @@ -123,6 +129,8 @@ static NTSTATUS NTAPI StreamSocketConnectComplete Status = MakeSocketIntoConnection( FCB ); if( !NT_SUCCESS(Status) ) { + Irp->IoStatus.Status = Status; + Irp->IoStatus.Information = 0; SocketStateUnlock( FCB ); return Status; } @@ -143,6 +151,12 @@ static NTSTATUS NTAPI StreamSocketConnectComplete Status = STATUS_SUCCESS; } + if (!NT_SUCCESS(Status)) + { + Irp->IoStatus.Status = Status; + Irp->IoStatus.Information = 0; + } + SocketStateUnlock( FCB ); AFD_DbgPrint(MID_TRACE,("Returning %x\n", Status)); diff --git a/reactos/drivers/network/afd/afd/listen.c b/reactos/drivers/network/afd/afd/listen.c index da5666708bf..e449b09eb18 100644 --- a/reactos/drivers/network/afd/afd/listen.c +++ b/reactos/drivers/network/afd/afd/listen.c @@ -112,18 +112,10 @@ static NTSTATUS NTAPI ListenComplete FCB->ListenIrp.InFlightRequest = NULL; - if( Irp->Cancel ) { - Irp->IoStatus.Status = STATUS_CANCELLED; - Irp->IoStatus.Information = 0; - SocketStateUnlock( FCB ); - return STATUS_CANCELLED; - } - if( FCB->State == SOCKET_STATE_CLOSED ) { Irp->IoStatus.Status = STATUS_FILE_CLOSED; Irp->IoStatus.Information = 0; SocketStateUnlock( FCB ); - DestroySocket( FCB ); return STATUS_FILE_CLOSED; } diff --git a/reactos/drivers/network/afd/afd/main.c b/reactos/drivers/network/afd/afd/main.c index b107904fb55..25d5dec09ac 100644 --- a/reactos/drivers/network/afd/afd/main.c +++ b/reactos/drivers/network/afd/afd/main.c @@ -180,36 +180,20 @@ VOID DestroySocket( PAFD_FCB FCB ) { InFlightRequest[2] = &FCB->SendIrp; InFlightRequest[3] = &FCB->ConnectIrp; - /* Return early here because we might be called in the mean time. */ - if( FCB->Critical || - FCB->ListenIrp.InFlightRequest || - FCB->ReceiveIrp.InFlightRequest || - FCB->SendIrp.InFlightRequest || - FCB->ConnectIrp.InFlightRequest ) { - AFD_DbgPrint(MIN_TRACE,("Leaving socket alive (%x %x %x %x)\n", - FCB->ListenIrp.InFlightRequest, - FCB->ReceiveIrp.InFlightRequest, - FCB->SendIrp.InFlightRequest, - FCB->ConnectIrp.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++ ) { if( InFlightRequest[i]->InFlightRequest ) { AFD_DbgPrint(MID_TRACE,("Cancelling in flight irp %d (%x)\n", i, InFlightRequest[i]->InFlightRequest)); - IoCancelIrp(InFlightRequest[i]->InFlightRequest); - InFlightRequest[i]->InFlightRequest = NULL; + if (!IoCancelIrp(InFlightRequest[i]->InFlightRequest)) + ReturnEarly = TRUE; } } SocketStateUnlock( FCB ); - if( ReturnEarly ) return; + if( ReturnEarly ) + return; if( FCB->Recv.Window ) ExFreePool( FCB->Recv.Window ); diff --git a/reactos/drivers/network/afd/afd/read.c b/reactos/drivers/network/afd/afd/read.c index 445674791b0..31d3339711b 100644 --- a/reactos/drivers/network/afd/afd/read.c +++ b/reactos/drivers/network/afd/afd/read.c @@ -238,13 +238,6 @@ NTSTATUS NTAPI ReceiveComplete FCB->ReceiveIrp.InFlightRequest = NULL; - if( Irp->Cancel ) { - Irp->IoStatus.Status = STATUS_CANCELLED; - Irp->IoStatus.Information = 0; - SocketStateUnlock( FCB ); - return STATUS_CANCELLED; - } - FCB->Recv.Content = Irp->IoStatus.Information; FCB->Recv.BytesUsed = 0; @@ -253,7 +246,6 @@ NTSTATUS NTAPI ReceiveComplete Irp->IoStatus.Status = STATUS_FILE_CLOSED; Irp->IoStatus.Information = 0; SocketStateUnlock( FCB ); - DestroySocket( FCB ); return STATUS_FILE_CLOSED; } else if( FCB->State == SOCKET_STATE_LISTENING ) { AFD_DbgPrint(MIN_TRACE,("!!! LISTENER GOT A RECEIVE COMPLETE !!!\n")); @@ -468,18 +460,10 @@ PacketSocketRecvComplete( FCB->ReceiveIrp.InFlightRequest = NULL; - if( Irp->IoStatus.Status == STATUS_CANCELLED ) { - Irp->IoStatus.Status = STATUS_CANCELLED; - Irp->IoStatus.Information = 0; - SocketStateUnlock( FCB ); - return STATUS_CANCELLED; - } - if( FCB->State == SOCKET_STATE_CLOSED ) { Irp->IoStatus.Status = STATUS_FILE_CLOSED; Irp->IoStatus.Information = 0; SocketStateUnlock( FCB ); - DestroySocket( FCB ); return STATUS_FILE_CLOSED; } diff --git a/reactos/drivers/network/afd/afd/write.c b/reactos/drivers/network/afd/afd/write.c index 160e89534ce..a4461387dd6 100644 --- a/reactos/drivers/network/afd/afd/write.c +++ b/reactos/drivers/network/afd/afd/write.c @@ -49,18 +49,10 @@ static NTSTATUS NTAPI SendComplete FCB->SendIrp.InFlightRequest = NULL; /* Request is not in flight any longer */ - if( Irp->Cancel ) { - Irp->IoStatus.Status = STATUS_CANCELLED; - Irp->IoStatus.Information = 0; - SocketStateUnlock( FCB ); - return STATUS_CANCELLED; - } - if( FCB->State == SOCKET_STATE_CLOSED ) { Irp->IoStatus.Status = STATUS_FILE_CLOSED; Irp->IoStatus.Information = 0; SocketStateUnlock( FCB ); - DestroySocket( FCB ); return STATUS_FILE_CLOSED; } @@ -189,13 +181,6 @@ static NTSTATUS NTAPI PacketSocketSendComplete FCB->SendIrp.InFlightRequest = NULL; /* Request is not in flight any longer */ - if( Irp->Cancel ) { - Irp->IoStatus.Status = STATUS_CANCELLED; - Irp->IoStatus.Information = 0; - SocketStateUnlock( FCB ); - return STATUS_CANCELLED; - } - FCB->PollState |= AFD_EVENT_SEND; PollReeval( FCB->DeviceExt, FCB->FileObject ); @@ -203,7 +188,6 @@ static NTSTATUS NTAPI PacketSocketSendComplete Irp->IoStatus.Status = STATUS_FILE_CLOSED; Irp->IoStatus.Information = 0; SocketStateUnlock( FCB ); - DestroySocket( FCB ); return STATUS_FILE_CLOSED; }