- Finally get IRP_MJ_CLOSE working properly

- Remove handling of IRP_MJ_CLEANUP and move the code to the DispatchClose routine
 - Remove the hack (holding an extra reference and not closing the handle) which hid these bugs
 - Fixes some memory and handle leaks too

svn path=/trunk/; revision=42387

reactos/drivers/network/afd/afd/bind.c

@@ -33,11 +33,6 @@ NTSTATUS WarmSocketForBind( PAFD_FCB FCB ) {
                                 &FCB->AddressFile.Handle,
                                 &FCB->AddressFile.Object );
 
-	if (NT_SUCCESS(Status)) 
-	{
-		ObReferenceObject(FCB->AddressFile.Object);
-	}
-
     AFD_DbgPrint(MID_TRACE,("Returning %x\n", Status));
 
     return Status;

reactos/drivers/network/afd/afd/connect.c

@@ -29,11 +29,6 @@ NTSTATUS WarmSocketForConnection( PAFD_FCB FCB ) {
                                           FCB->Connection.Object );
     }
 
-	if (NT_SUCCESS(Status)) 
-	{
-		ObReferenceObject(FCB->Connection.Object);
-	}
-
     return Status;
 }
 

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

@@ -96,6 +96,8 @@ AfdCreateSocket(PDEVICE_OBJECT DeviceObject, PIRP Irp,
     FCB->DeviceExt = DeviceExt;
     FCB->Recv.Size = DEFAULT_RECEIVE_WINDOW_SIZE;
     FCB->Send.Size = DEFAULT_SEND_WINDOW_SIZE;
+    FCB->AddressFile.Handle = INVALID_HANDLE_VALUE;
+    FCB->Connection.Handle = INVALID_HANDLE_VALUE;
 
     KeInitializeSpinLock( &FCB->SpinLock );
     ExInitializeFastMutex( &FCB->Mutex );
@@ -167,13 +169,19 @@ AfdCreateSocket(PDEVICE_OBJECT DeviceObject, PIRP Irp,
     return Status;
 }
 
-VOID CleanupSocket( PAFD_FCB FCB ) {
+static NTSTATUS NTAPI
+AfdCloseSocket(PDEVICE_OBJECT DeviceObject, PIRP Irp,
+	       PIO_STACK_LOCATION IrpSp)
+{
+    PFILE_OBJECT FileObject = IrpSp->FileObject;
+    PAFD_FCB FCB = FileObject->FsContext;
     UINT i;
     PAFD_IN_FLIGHT_REQUEST InFlightRequest[IN_FLIGHT_REQUESTS];
 
-    AFD_DbgPrint(MIN_TRACE,("Called (%x)\n", FCB));
+    AFD_DbgPrint(MID_TRACE,
+		 ("AfdClose(DeviceObject %p Irp %p)\n", DeviceObject, Irp));
 
-    if( !SocketAcquireStateLock( FCB ) ) return;
+    if( !SocketAcquireStateLock( FCB ) ) return STATUS_FILE_CLOSED;
 
     FCB->State = SOCKET_STATE_CLOSED;
 
@@ -191,76 +199,47 @@ VOID CleanupSocket( PAFD_FCB FCB ) {
 	}
     }
 
-    FCB->State = SOCKET_STATE_CREATED;
-
-    if( FCB->EventSelect ) {
-        ObDereferenceObject( FCB->EventSelect );
-        FCB->EventSelect = NULL;
-    }
-    if( FCB->Context ) {
-        ExFreePool( FCB->Context );
-        FCB->Context = NULL;
-    }
-    if( FCB->Recv.Window ) {
-	ExFreePool( FCB->Recv.Window );
-        FCB->Recv.Window = NULL;
-    }
-    if( FCB->Send.Window ) {
-	ExFreePool( FCB->Send.Window );
-        FCB->Send.Window = NULL;
-    }
-    if( FCB->AddressFrom ) {
-	ExFreePool( FCB->AddressFrom );
-        FCB->AddressFrom = NULL;
-    }
-    if( FCB->LocalAddress ) {
-	ExFreePool( FCB->LocalAddress );
-        FCB->LocalAddress = NULL;
-    }
-    if( FCB->RemoteAddress ) {
-	ExFreePool( FCB->RemoteAddress );
-        FCB->RemoteAddress = NULL;
-    }
-    if( FCB->Connection.Object ) {
-	ZwClose(FCB->Connection.Handle);
-	ObDereferenceObject(FCB->Connection.Object);
-        FCB->Connection.Object = NULL;
-    }
-    if( FCB->AddressFile.Object ) {
-	ZwClose(FCB->AddressFile.Handle);
-	ObDereferenceObject(FCB->AddressFile.Object);
-        FCB->AddressFile.Object = NULL;
-    }
+    KillSelectsForFCB( FCB->DeviceExt, FileObject, FALSE );
 
     SocketStateUnlock( FCB );
-}
 
-VOID DestroySocket( PAFD_FCB FCB ) {
+    if( FCB->EventSelect )
+        ObDereferenceObject( FCB->EventSelect );
+
+    if( FCB->Context )
+        ExFreePool( FCB->Context );
+
+    if( FCB->Recv.Window )
+	ExFreePool( FCB->Recv.Window );
+
+    if( FCB->Send.Window )
+	ExFreePool( FCB->Send.Window );
+
+    if( FCB->AddressFrom )
+	ExFreePool( FCB->AddressFrom );
+
+    if( FCB->LocalAddress )
+	ExFreePool( FCB->LocalAddress );
+
+    if( FCB->RemoteAddress )
+	ExFreePool( FCB->RemoteAddress );
+
+    if( FCB->Connection.Object )
+	ObDereferenceObject(FCB->Connection.Object);
+
+    if( FCB->AddressFile.Object )
+	ObDereferenceObject(FCB->AddressFile.Object);
+
+    if( FCB->AddressFile.Handle != INVALID_HANDLE_VALUE )
+        ZwClose(FCB->AddressFile.Handle);
+
+    if( FCB->Connection.Handle != INVALID_HANDLE_VALUE )
+        ZwClose(FCB->Connection.Handle);
+
     if( FCB->TdiDeviceName.Buffer )
 	ExFreePool(FCB->TdiDeviceName.Buffer);
 
     ExFreePool(FCB);
-    AFD_DbgPrint(MIN_TRACE,("Deleted (%x)\n", FCB));
-}
-
-static NTSTATUS NTAPI
-AfdCloseSocket(PDEVICE_OBJECT DeviceObject, PIRP Irp,
-	       PIO_STACK_LOCATION IrpSp)
-{
-    PFILE_OBJECT FileObject = IrpSp->FileObject;
-    PAFD_FCB FCB = FileObject->FsContext;
-
-    AFD_DbgPrint(MID_TRACE,
-		 ("AfdClose(DeviceObject %p Irp %p)\n", DeviceObject, Irp));
-
-    if( !SocketAcquireStateLock( FCB ) ) return LostSocket( Irp );
-
-    AFD_DbgPrint(MID_TRACE,("FCB %x\n", FCB));
-
-    FileObject->FsContext = NULL;
-    SocketStateUnlock( FCB );
-
-    DestroySocket( FCB );
 
     Irp->IoStatus.Status = STATUS_SUCCESS;
     Irp->IoStatus.Information = 0;
@@ -271,28 +250,6 @@ AfdCloseSocket(PDEVICE_OBJECT DeviceObject, PIRP Irp,
     return STATUS_SUCCESS;
 }
 
-static NTSTATUS NTAPI
-AfdCleanupSocket(PDEVICE_OBJECT DeviceObject, PIRP Irp,
-                 PIO_STACK_LOCATION IrpSp)
-{
-    PFILE_OBJECT FileObject = IrpSp->FileObject;
-    PAFD_FCB FCB = FileObject->FsContext;
-
-    if( !SocketAcquireStateLock( FCB ) ) return LostSocket( Irp );
-
-    CleanupSocket( FCB );
-
-    KillSelectsForFCB( FCB->DeviceExt, FileObject, FALSE );
-
-    SocketStateUnlock( FCB );
-
-    Irp->IoStatus.Status = STATUS_SUCCESS;
-    Irp->IoStatus.Information = 0;
-    IoCompleteRequest(Irp, IO_NETWORK_INCREMENT);
-
-    return STATUS_SUCCESS;
-}
-
 static NTSTATUS NTAPI
 AfdDisconnect(PDEVICE_OBJECT DeviceObject, PIRP Irp,
 	      PIO_STACK_LOCATION IrpSp) {
@@ -373,9 +330,6 @@ AfdDispatch(PDEVICE_OBJECT DeviceObject, PIRP Irp)
 	/* Ditto the borrowing */
 	return AfdCloseSocket(DeviceObject, Irp, IrpSp);
 
-    case IRP_MJ_CLEANUP:
-        return AfdCleanupSocket(DeviceObject, Irp, IrpSp);
-
     /* write data */
     case IRP_MJ_WRITE:
 	return AfdConnectedSocketWriteData( DeviceObject, Irp, IrpSp, TRUE );
@@ -550,7 +504,6 @@ DriverEntry(PDRIVER_OBJECT DriverObject, PUNICODE_STRING RegistryPath)
     /* register driver routines */
     DriverObject->MajorFunction[IRP_MJ_CLOSE] = AfdDispatch;
     DriverObject->MajorFunction[IRP_MJ_CREATE] = AfdDispatch;
-    DriverObject->MajorFunction[IRP_MJ_CLEANUP] = AfdDispatch;
     DriverObject->MajorFunction[IRP_MJ_WRITE] = AfdDispatch;
     DriverObject->MajorFunction[IRP_MJ_READ] = AfdDispatch;
     DriverObject->MajorFunction[IRP_MJ_DEVICE_CONTROL] = AfdDispatch;

reactos/drivers/network/afd/afd/tdi.c

@@ -146,14 +146,13 @@ static NTSTATUS TdiOpenDevice(
     }
 
     if (!NT_SUCCESS(Status)) {
-        *Handle = NULL;
+        *Handle = INVALID_HANDLE_VALUE;
         *Object = NULL;
     }
 
     return Status;
 }
 
-
 NTSTATUS TdiOpenAddressFile(
     PUNICODE_STRING DeviceName,
     PTRANSPORT_ADDRESS Name,

reactos/drivers/network/tcpip/include/fileobjs.h

@@ -23,9 +23,6 @@ NTSTATUS FileOpenAddress(
 NTSTATUS FileCloseAddress(
   PTDI_REQUEST Request);
 
-NTSTATUS FileFreeAddress(
-  PTDI_REQUEST Request);
-
 NTSTATUS FileOpenConnection(
   PTDI_REQUEST Request,
   PVOID ClientContext);
@@ -35,13 +32,10 @@ PCONNECTION_ENDPOINT FileFindConnectionByContext( PVOID Context );
 NTSTATUS FileCloseConnection(
   PTDI_REQUEST Request);
 
-NTSTATUS FileFreeConnection(
-  PTDI_REQUEST Request);
-
 NTSTATUS FileOpenControlChannel(
   PTDI_REQUEST Request);
 
-NTSTATUS FileFreeControlChannel(
+NTSTATUS FileCloseControlChannel(
   PTDI_REQUEST Request);
 
 #endif /* __FILEOBJS_H */

reactos/drivers/network/tcpip/tcpip/fileobjs.c

@@ -284,17 +284,21 @@ NTSTATUS FileOpenAddress(
 NTSTATUS FileCloseAddress(
   PTDI_REQUEST Request)
 {
-  KIRQL OldIrql;
   PADDRESS_FILE AddrFile;
   NTSTATUS Status = STATUS_SUCCESS;
-  PDATAGRAM_SEND_REQUEST SendRequest;
+  KIRQL OldIrql;
   PDATAGRAM_RECEIVE_REQUEST ReceiveRequest;
-  PLIST_ENTRY CurrentEntry;
-  PLIST_ENTRY NextEntry;
+  PDATAGRAM_SEND_REQUEST SendRequest;
+  PLIST_ENTRY CurrentEntry, NextEntry;
+
+  AddrFile = Request->Handle.AddressHandle;
 
   TI_DbgPrint(MID_TRACE, ("Called.\n"));
 
-  AddrFile = Request->Handle.AddressHandle;
+  /* Remove address file from the global list */
+  TcpipAcquireSpinLock(&AddressFileListLock, &OldIrql);
+  RemoveEntryList(&AddrFile->ListEntry);
+  TcpipReleaseSpinLock(&AddressFileListLock, OldIrql);
 
   TcpipAcquireSpinLock(&AddrFile->Lock, &OldIrql);
 
@@ -311,7 +315,7 @@ NTSTATUS FileCloseAddress(
     ReceiveRequest = CONTAINING_RECORD(CurrentEntry, DATAGRAM_RECEIVE_REQUEST, ListEntry);
     /* Abort the request and free its resources */
     TcpipReleaseSpinLock(&AddrFile->Lock, OldIrql);
-    (*ReceiveRequest->Complete)(ReceiveRequest->Context, STATUS_ADDRESS_CLOSED, 0);
+    (*ReceiveRequest->Complete)(ReceiveRequest->Context, STATUS_CANCELLED, 0);
     TcpipAcquireSpinLock(&AddrFile->Lock, &OldIrql);
     CurrentEntry = NextEntry;
   }
@@ -326,7 +330,7 @@ NTSTATUS FileCloseAddress(
 				    DATAGRAM_SEND_REQUEST, ListEntry);
     /* Abort the request and free its resources */
     TcpipReleaseSpinLock(&AddrFile->Lock, OldIrql);
-    (*SendRequest->Complete)(SendRequest->Context, STATUS_ADDRESS_CLOSED, 0);
+    (*SendRequest->Complete)(SendRequest->Context, STATUS_CANCELLED, 0);
     exFreePool(SendRequest);
     TcpipAcquireSpinLock(&AddrFile->Lock, &OldIrql);
     CurrentEntry = NextEntry;
@@ -334,35 +338,6 @@ NTSTATUS FileCloseAddress(
 
   TcpipReleaseSpinLock(&AddrFile->Lock, OldIrql);
 
-  TI_DbgPrint(MAX_TRACE, ("Leaving.\n"));
-
-  return Status;
-}
-
-
-/*
- * FUNCTION: Closes an address file object
- * ARGUMENTS:
- *     Request = Pointer to TDI request structure for this request
- * RETURNS:
- *     Status of operation
- */
-NTSTATUS FileFreeAddress(
-  PTDI_REQUEST Request)
-{
-  PADDRESS_FILE AddrFile;
-  NTSTATUS Status = STATUS_SUCCESS;
-  KIRQL OldIrql;
-
-  AddrFile = Request->Handle.AddressHandle;
-
-  TI_DbgPrint(MID_TRACE, ("Called.\n"));
-
-  /* Remove address file from the global list */
-  TcpipAcquireSpinLock(&AddressFileListLock, &OldIrql);
-  RemoveEntryList(&AddrFile->ListEntry);
-  TcpipReleaseSpinLock(&AddressFileListLock, OldIrql);
-
   /* Protocol specific handling */
   switch (AddrFile->Protocol) {
   case IPPROTO_TCP:
@@ -474,33 +449,7 @@ NTSTATUS FileCloseConnection(
   PTDI_REQUEST Request)
 {
   PCONNECTION_ENDPOINT Connection;
-
-  TI_DbgPrint(MID_TRACE, ("Called.\n"));
-
-  Connection = Request->Handle.ConnectionContext;
-
-  TcpipRecursiveMutexEnter( &TCPLock, TRUE );
-  TCPClose( Connection );
-  TcpipRecursiveMutexLeave( &TCPLock );
-
-  TI_DbgPrint(MAX_TRACE, ("Leaving.\n"));
-
-  return STATUS_SUCCESS;
-}
-
-
-/*
- * FUNCTION: Frees an connection file object
- * ARGUMENTS:
- *     Request = Pointer to TDI request structure for this request
- * RETURNS:
- *     Status of operation
- */
-NTSTATUS FileFreeConnection(
-  PTDI_REQUEST Request)
-{
   KIRQL OldIrql;
-  PCONNECTION_ENDPOINT Connection;
 
   TI_DbgPrint(MID_TRACE, ("Called.\n"));
 
@@ -510,6 +459,10 @@ NTSTATUS FileFreeConnection(
   RemoveEntryList(&Connection->ListEntry);
   TcpipReleaseSpinLock(&ConnectionEndpointListLock, OldIrql);
 
+  TcpipRecursiveMutexEnter( &TCPLock, TRUE );
+  TCPClose( Connection );
+  TcpipRecursiveMutexLeave( &TCPLock );
+
   TCPFreeConnectionEndpoint(Connection);
 
   TI_DbgPrint(MAX_TRACE, ("Leaving.\n"));
@@ -565,7 +518,7 @@ NTSTATUS FileOpenControlChannel(
  * RETURNS:
  *     Status of operation
  */
-NTSTATUS FileFreeControlChannel(
+NTSTATUS FileCloseControlChannel(
   PTDI_REQUEST Request)
 {
   PCONTROL_CHANNEL ControlChannel = Request->Handle.ControlChannel;

reactos/drivers/network/tcpip/tcpip/main.c

@@ -252,74 +252,6 @@ CP
   return Status;
 }
 
-/*
- * FUNCTION: Prepares a file object for close
- * ARGUMENTS:
- *     DeviceObject = Pointer to a device object for this driver
- *     Irp          = Pointer to a I/O request packet
- * RETURNS:
- *     Status of the operation
- * NOTES:
- *     This function does not pend
- */
-NTSTATUS TiCleanupFileObject(
-  PDEVICE_OBJECT DeviceObject,
-  PIRP Irp)
-{
-  PIO_STACK_LOCATION IrpSp;
-  PTRANSPORT_CONTEXT Context;
-  TDI_REQUEST Request;
-  NTSTATUS Status;
-  KIRQL OldIrql;
-
-  IrpSp   = IoGetCurrentIrpStackLocation(Irp);
-  Context = IrpSp->FileObject->FsContext;
-  if (!Context) {
-    TI_DbgPrint(MIN_TRACE, ("Parameters are invalid.\n"));
-    return STATUS_INVALID_PARAMETER;
-  }
-
-  IoAcquireCancelSpinLock(&OldIrql);
-
-  Context->CancelIrps = TRUE;
-
-  IoReleaseCancelSpinLock(OldIrql);
-
-  switch ((ULONG_PTR)IrpSp->FileObject->FsContext2) {
-  case TDI_TRANSPORT_ADDRESS_FILE:
-    Request.Handle.AddressHandle = Context->Handle.AddressHandle;
-    Status = FileCloseAddress(&Request);
-    break;
-
-  case TDI_CONNECTION_FILE:
-    Request.Handle.ConnectionContext = Context->Handle.ConnectionContext;
-    Status = FileCloseConnection(&Request);
-    break;
-
-  case TDI_CONTROL_CHANNEL_FILE:
-    Request.Handle.ControlChannel = Context->Handle.ControlChannel;
-	/* Nothing to do to close */
-	Status = STATUS_SUCCESS;
-    break;
-
-  default:
-    /* This should never happen */
-
-    TI_DbgPrint(MIN_TRACE, ("Unknown transport context.\n"));
-
-    IoAcquireCancelSpinLock(&OldIrql);
-    Context->CancelIrps = FALSE;
-    IoReleaseCancelSpinLock(OldIrql);
-
-    Status = STATUS_INVALID_PARAMETER;
-
-  }
-
-  Irp->IoStatus.Status = Status;
-
-  return Irp->IoStatus.Status;
-}
-
 
 /*
  * FUNCTION: Releases resources used by a file object
@@ -339,7 +271,6 @@ NTSTATUS TiCloseFileObject(
   PTRANSPORT_CONTEXT Context;
   TDI_REQUEST Request;
   NTSTATUS Status;
-  KIRQL OldIrql;
 
   IrpSp   = IoGetCurrentIrpStackLocation(Irp);
   Context = IrpSp->FileObject->FsContext;
@@ -348,39 +279,26 @@ NTSTATUS TiCloseFileObject(
     return STATUS_INVALID_PARAMETER;
   }
 
-  IoAcquireCancelSpinLock(&OldIrql);
-
-  Context->CancelIrps = TRUE;
-
-  IoReleaseCancelSpinLock(OldIrql);
-
   switch ((ULONG_PTR)IrpSp->FileObject->FsContext2) {
   case TDI_TRANSPORT_ADDRESS_FILE:
     Request.Handle.AddressHandle = Context->Handle.AddressHandle;
-    Status = FileFreeAddress(&Request);
+    Status = FileCloseAddress(&Request);
     break;
 
   case TDI_CONNECTION_FILE:
     Request.Handle.ConnectionContext = Context->Handle.ConnectionContext;
-    Status = FileFreeConnection(&Request);
+    Status = FileCloseConnection(&Request);
     break;
 
   case TDI_CONTROL_CHANNEL_FILE:
     Request.Handle.ControlChannel = Context->Handle.ControlChannel;
-    Status = FileFreeControlChannel(&Request);
+    Status = FileCloseControlChannel(&Request);
     break;
 
   default:
-    /* This should never happen */
-
-    TI_DbgPrint(MIN_TRACE, ("Unknown transport context.\n"));
-
-    IoAcquireCancelSpinLock(&OldIrql);
-    Context->CancelIrps = FALSE;
-    IoReleaseCancelSpinLock(OldIrql);
-
+    DbgPrint("Unknown type %d\n", (ULONG_PTR)IrpSp->FileObject->FsContext2);
     Status = STATUS_INVALID_PARAMETER;
-
+    break;
   }
 
   Irp->IoStatus.Status = Status;
@@ -424,12 +342,6 @@ TiDispatchOpenClose(
 	Status = TiCloseFileObject(DeviceObject, Irp);
     break;
 
-  /* Release resources bound to an address file, connection endpoint,
-     or control connection */
-  case IRP_MJ_CLEANUP:
-    Status = TiCleanupFileObject(DeviceObject, Irp);
-    break;
-
   default:
     Status = STATUS_INVALID_DEVICE_REQUEST;
   }
@@ -912,7 +824,6 @@ DriverEntry(
   /* Initialize the driver object with this driver's entry points */
   DriverObject->MajorFunction[IRP_MJ_CREATE]  = TiDispatchOpenClose;
   DriverObject->MajorFunction[IRP_MJ_CLOSE]   = TiDispatchOpenClose;
-  DriverObject->MajorFunction[IRP_MJ_CLEANUP] = TiDispatchOpenClose;
   DriverObject->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = TiDispatchInternal;
   DriverObject->MajorFunction[IRP_MJ_DEVICE_CONTROL] = TiDispatch;