IRP Completion Fixes for 2nd-Stage:
- Free ALL Mdls, not just the first
- Don't copy buffered data just because the Device Object is buffered.
Check if the IRP is.
- Don't handle MajorFunctions differenty, use flags which are now
correctly set in order to determine course of action.
- Free memory by using flag.
- Don't remove IRP from Thread List too soon.
- Don't use FileObject fields after dereferencing it.
- Don't call IO Completion if there is already an APC routine.
- Signal FileObject/UserEvent properly depending on cases.
- Don't call UserAPC and set events like before if the IRP actually
failed.
Modified: trunk/reactos/ntoskrnl/io/file.c
Modified: trunk/reactos/ntoskrnl/io/irp.c
_____
Modified: trunk/reactos/ntoskrnl/io/file.c
--- trunk/reactos/ntoskrnl/io/file.c 2005-04-29 11:25:55 UTC (rev
14860)
+++ trunk/reactos/ntoskrnl/io/file.c 2005-04-29 14:38:05 UTC (rev
14861)
@@ -500,13 +500,18 @@
KeResetEvent( &FileObject->Event );
+ IO_STATUS_BLOCK Dummy;
+ /* WRONG WRONG WRONG WRONG!!!!!! */
Irp = IoBuildSynchronousFsdRequest(IRP_MJ_CLEANUP,
FileObject->DeviceObject,
NULL,
0,
NULL,
&FileObject->Event,
- NULL);
+ &Dummy);
+ /* Hack to fix the above WRONG WRONG WRONG WRONG CODE!!! */
+ Irp->UserIosb = &Irp->IoStatus;
+
StackPtr = IoGetNextIrpStackLocation(Irp);
StackPtr->FileObject = FileObject;
@@ -890,6 +895,7 @@
* immediately.
*/
Status = IofCallDriver(FileObject->DeviceObject, Irp );
+ DPRINT1("Status :%x\n", Status);
if (Status == STATUS_PENDING)
{
_____
Modified: trunk/reactos/ntoskrnl/io/irp.c
--- trunk/reactos/ntoskrnl/io/irp.c 2005-04-29 11:25:55 UTC (rev
14860)
+++ trunk/reactos/ntoskrnl/io/irp.c 2005-04-29 14:38:05 UTC (rev
14861)
@@ -132,7 +132,7 @@
else
{
/* Set the Input Operation flag and set this as a User
Buffer */
- Irp->Flags = IRP_INPUT_OPERATION;
+ Irp->Flags |= IRP_INPUT_OPERATION;
Irp->UserBuffer = Buffer;
}
}
@@ -189,6 +189,7 @@
}
/* Set the Current Thread and IOSB */
+ if (!IoStatusBlock) KEBUGCHECK(0); /* Temporary to catch illegal
ROS Drivers */
Irp->UserIosb = IoStatusBlock;
Irp->Tail.Overlay.Thread = PsGetCurrentThread();
@@ -391,6 +392,7 @@
}
/* Now write the Event and IoSB */
+ if (!IoStatusBlock) KEBUGCHECK(0); /* Temporary to catch illegal
ROS Drivers */
Irp->UserIosb = IoStatusBlock;
Irp->UserEvent = Event;
@@ -1172,309 +1174,223 @@
}
VOID
-IoDeviceControlCompletion(PDEVICE_OBJECT DeviceObject,
- PIRP Irp,
- PIO_STACK_LOCATION IoStack)
+STDCALL
+IoSecondStageCompletion_KernelApcRoutine(PKAPC Apc,
+ PKNORMAL_ROUTINE
*NormalRoutine,
+ PVOID *NormalContext,
+ PVOID *SystemArgument1,
+ PVOID *SystemArgument2)
{
- ULONG IoControlCode;
- ULONG OutputBufferLength;
-
- if (IoStack->MajorFunction == IRP_MJ_FILE_SYSTEM_CONTROL)
- {
- IoControlCode =
- IoStack->Parameters.FileSystemControl.FsControlCode;
- OutputBufferLength =
- IoStack->Parameters.FileSystemControl.OutputBufferLength;
- }
- else
- {
- IoControlCode =
IoStack->Parameters.DeviceIoControl.IoControlCode;
-
- if (NT_SUCCESS(Irp->IoStatus.Status))
- {
- OutputBufferLength = Irp->IoStatus.Information;
-
- if (IoStack->Parameters.DeviceIoControl.OutputBufferLength <
OutputBufferLength)
- {
- OutputBufferLength =
IoStack->Parameters.DeviceIoControl.OutputBufferLength;
- }
- }
- else
- {
- OutputBufferLength = 0;
- }
- }
-
- switch (IO_METHOD_FROM_CTL_CODE(IoControlCode))
- {
- case METHOD_BUFFERED:
-
- /* copy output buffer back and free it */
- if (Irp->AssociatedIrp.SystemBuffer)
- {
- if (OutputBufferLength)
- {
- RtlCopyMemory(Irp->UserBuffer,
- Irp->AssociatedIrp.SystemBuffer,
- OutputBufferLength);
- }
- ExFreePool (Irp->AssociatedIrp.SystemBuffer);
- }
- break;
-
- case METHOD_IN_DIRECT:
- DPRINT ("Using METHOD_IN_DIRECT!\n");
- /* use the same code as for METHOD_OUT_DIRECT */
-
- case METHOD_OUT_DIRECT:
- DPRINT ("Using METHOD_OUT_DIRECT!\n");
-
- /* free input buffer (control buffer) */
- if (Irp->AssociatedIrp.SystemBuffer)
- ExFreePool (Irp->AssociatedIrp.SystemBuffer);
-
- /* free output buffer (data transfer buffer) */
- if (Irp->MdlAddress)
- IoFreeMdl (Irp->MdlAddress);
- break;
-
- case METHOD_NEITHER:
- DPRINT ("Using METHOD_NEITHER!\n");
- /* nothing to do */
- break;
- }
+ /* Free the IRP */
+ IoFreeIrp(CONTAINING_RECORD(Apc, IRP, Tail.Apc));
}
-VOID IoReadWriteCompletion(PDEVICE_OBJECT DeviceObject,
- PIRP Irp,
- PIO_STACK_LOCATION IoStack)
-{
- PFILE_OBJECT FileObject;
-
- FileObject = IoStack->FileObject;
-
- if (DeviceObject->Flags & DO_BUFFERED_IO)
- {
- if (IoStack->MajorFunction == IRP_MJ_READ)
- {
- DPRINT("Copying buffered io back to user\n");
- memcpy(Irp->UserBuffer,Irp->AssociatedIrp.SystemBuffer,
- IoStack->Parameters.Read.Length);
- }
- ExFreePool(Irp->AssociatedIrp.SystemBuffer);
- }
-
- if (DeviceObject->Flags & DO_DIRECT_IO)
- {
- if (Irp->MdlAddress)
- {
- IoFreeMdl(Irp->MdlAddress);
- }
- }
-}
-
-VOID IoVolumeInformationCompletion(PDEVICE_OBJECT DeviceObject,
- PIRP Irp,
- PIO_STACK_LOCATION IoStack)
+VOID
+STDCALL
+IoSecondStageCompletion_RundownApcRoutine(PKAPC Apc)
{
+ /* Free the IRP */
+ IoFreeIrp(CONTAINING_RECORD(Apc, IRP, Tail.Apc));
}
-
-VOID STDCALL
-IoSecondStageCompletion_KernelApcRoutine(
- IN PKAPC Apc,
- IN OUT PKNORMAL_ROUTINE *NormalRoutine,
- IN OUT PVOID *NormalContext,
- IN OUT PVOID *SystemArgument1,
- IN OUT PVOID *SystemArgument2
- )
-{
- PIRP Irp;
-
- Irp = CONTAINING_RECORD(Apc, IRP, Tail.Apc);
- IoFreeIrp(Irp);
-}
-
-
-VOID STDCALL
-IoSecondStageCompletion_RundownApcRoutine(
- IN PKAPC Apc
- )
-{
- PIRP Irp;
-
- Irp = CONTAINING_RECORD(Apc, IRP, Tail.Apc);
- IoFreeIrp(Irp);
-}
-
-
/*
* FUNCTION: Performs the second stage of irp completion for read/write
irps
*
* Called as a special kernel APC kernel-routine or directly from
IofCompleteRequest()
+ *
+ * Note that we'll never see irp's flagged IRP_PAGING_IO
(IRP_MOUNT_OPERATION)
+ * or IRP_CLOSE_OPERATION (IRP_MJ_CLOSE and IRP_MJ_CLEANUP) here since
their
+ * cleanup/completion is fully taken care of in IoCompleteRequest.
+ * -Gunnar
*/
-VOID STDCALL
-IoSecondStageCompletion(
- PKAPC Apc,
- PKNORMAL_ROUTINE* NormalRoutine,
- PVOID* NormalContext,
- PVOID* SystemArgument1,
- PVOID* SystemArgument2)
-
+VOID
+STDCALL
+IoSecondStageCompletion(PKAPC Apc,
+ PKNORMAL_ROUTINE* NormalRoutine,
+ PVOID* NormalContext,
+ PVOID* SystemArgument1,
+ PVOID* SystemArgument2)
{
- PIO_STACK_LOCATION IoStack;
- PDEVICE_OBJECT DeviceObject;
- PFILE_OBJECT OriginalFileObject;
- PIRP Irp;
+ PFILE_OBJECT FileObject;
+ PIRP Irp;
+ PMDL Mdl, NextMdl;
- if (Apc) DPRINT("IoSecondStageCompletition with APC: %x\n", Apc);
+ if (Apc) DPRINT("IoSecondStageCompletition with APC: %x\n", Apc);
- OriginalFileObject = (PFILE_OBJECT)(*SystemArgument1);
- DPRINT("OriginalFileObject: %x\n", OriginalFileObject);
+ /* Get data from the APC */
+ FileObject = (PFILE_OBJECT)(*SystemArgument1);
+ Irp = CONTAINING_RECORD(Apc, IRP, Tail.Apc);
+ DPRINT("IoSecondStageCompletition, %x\n", Irp);
+
+ /* Handle Buffered case first */
+ if (Irp->Flags & IRP_BUFFERED_IO)
+ {
+ /* Check if we have an input buffer and if we suceeded */
+ if (Irp->Flags & IRP_INPUT_OPERATION &&
NT_SUCCESS(Irp->IoStatus.Status))
+ {
+ /* Copy the buffer back to the user */
+ RtlCopyMemory(Irp->UserBuffer,
+ Irp->AssociatedIrp.SystemBuffer,
+ Irp->IoStatus.Information);
+ }
+
+ /* Also check if we should de-allocate it */
+ if (Irp->Flags & IRP_DEALLOCATE_BUFFER)
+ {
+ ExFreePool(Irp->AssociatedIrp.SystemBuffer);
+ }
+ }
+
+ /* Now we got rid of these two... */
+ Irp->Flags &= ~(IRP_BUFFERED_IO | IRP_DEALLOCATE_BUFFER);
+
+ /* Check if there's an MDL */
+ if ((Mdl = Irp->MdlAddress))
+ {
+ /* Clear all of them */
+ do
+ {
+ NextMdl = Mdl->Next;
+ IoFreeMdl(Mdl);
+ Mdl = NextMdl;
+ } while (Mdl);
+ }
+ Irp->MdlAddress = NULL;
+
+ /* Check for Success but allow failure for Async IRPs */
+ if (NT_SUCCESS(Irp->IoStatus.Status) ||
+ (Irp->PendingReturned &&
+ !(Irp->Flags & IRP_SYNCHRONOUS_API) &&
+ (FileObject == NULL || FileObject->Flags & FO_SYNCHRONOUS_IO)))
+ {
+ /* Save the IOSB Information */
+ *Irp->UserIosb = Irp->IoStatus;
+
+ /* Check if there's an event */
+ if (Irp->UserEvent)
+ {
+ /* Signal the Event */
+ KeSetEvent(Irp->UserEvent, 0, FALSE);
+
+ }
+ else if (FileObject)
+ {
+ /* Signal the File Object Instead */
+ KeSetEvent(&FileObject->Event, 0, FALSE);
+
+ /* Set the Status */
+ FileObject->FinalStatus = Irp->IoStatus.Status;
+ }
+
+ /* Check if there's a File Object */
+ if (FileObject)
+ {
+ /* Dereference the Event if this is an ASYNC IRP */
+ if (!Irp->Flags & IRP_SYNCHRONOUS_API)
+ {
+ ObDereferenceObject(Irp->UserEvent);
+ }
+
+ /* If the File Object is SYNC, then we need to signal its
event too */
+ if (FileObject->Flags & FO_SYNCHRONOUS_IO)
+ {
+ /* Signal Event */
- Irp = CONTAINING_RECORD(Apc, IRP, Tail.Apc);
- DPRINT("Irp: %x\n", Irp);
-
- /*
- * Note that we'll never see irp's flagged IRP_PAGING_IO
(IRP_MOUNT_OPERATION)
- * or IRP_CLOSE_OPERATION (IRP_MJ_CLOSE and IRP_MJ_CLEANUP) here
since their
- * cleanup/completion is fully taken care of in IoCompleteRequest.
- * -Gunnar
- */
+ KeSetEvent(&FileObject->Event, 0, FALSE);
+
+ /* Set the Status */
+ FileObject->FinalStatus = Irp->IoStatus.Status;
+ }
+ }
- /*
- Remove synchronous irp's from the threads cleanup list.
- To synchronize with the code inserting the entry, this code must run
- at APC_LEVEL
- */
- if (!IsListEmpty(&Irp->ThreadListEntry))
- {
- RemoveEntryList(&Irp->ThreadListEntry);
- InitializeListHead(&Irp->ThreadListEntry);
- }
-
- IoStack = (PIO_STACK_LOCATION)(Irp+1) + Irp->CurrentLocation;
- DeviceObject = IoStack->DeviceObject;
+ /* Remove the IRP from the list of Thread Pending IRPs */
+ RemoveEntryList(&Irp->ThreadListEntry);
+ InitializeListHead(&Irp->ThreadListEntry);
+
+ /* Now call the User APC if one was requested */
+ if (Irp->Overlay.AsynchronousParameters.UserApcRoutine != NULL)
+ {
+ KeInitializeApc(&Irp->Tail.Apc,
+ KeGetCurrentThread(),
+ CurrentApcEnvironment,
+ IoSecondStageCompletion_KernelApcRoutine,
+ IoSecondStageCompletion_RundownApcRoutine,
+
(PKNORMAL_ROUTINE)Irp->Overlay.AsynchronousParameters.UserApcRoutine,
+ Irp->RequestorMode,
+
Irp->Overlay.AsynchronousParameters.UserApcContext);
- DPRINT("IoSecondStageCompletion(Irp %x, MajorFunction %x)\n", Irp,
IoStack->MajorFunction);
-
- switch (IoStack->MajorFunction)
- {
- case IRP_MJ_CREATE:
- case IRP_MJ_FLUSH_BUFFERS:
- /* NOP */
- break;
-
- case IRP_MJ_READ:
- case IRP_MJ_WRITE:
- IoReadWriteCompletion(DeviceObject,Irp,IoStack);
- break;
-
- case IRP_MJ_DEVICE_CONTROL:
- case IRP_MJ_INTERNAL_DEVICE_CONTROL:
- case IRP_MJ_FILE_SYSTEM_CONTROL:
- IoDeviceControlCompletion(DeviceObject, Irp, IoStack);
- break;
-
- case IRP_MJ_QUERY_VOLUME_INFORMATION:
- case IRP_MJ_SET_VOLUME_INFORMATION:
- IoVolumeInformationCompletion(DeviceObject, Irp, IoStack);
- break;
-
- default:
- break;
- }
-
- if (Irp->UserIosb!=NULL)
- {
- if (Irp->RequestorMode == KernelMode)
- {
- *Irp->UserIosb = Irp->IoStatus;
- }
- else
- {
- DPRINT("Irp->RequestorMode == UserMode\n");
- MmSafeCopyToUser(Irp->UserIosb,
- &Irp->IoStatus,
- sizeof(IO_STATUS_BLOCK));
- }
- }
-
- if (Irp->UserEvent)
- {
- KeSetEvent(Irp->UserEvent,0,FALSE);
- }
-
- //Windows NT File System Internals, page 169
- if (OriginalFileObject)
- {
- if (Irp->UserEvent == NULL)
- {
- KeSetEvent(&OriginalFileObject->Event,0,FALSE);
- }
- else if (OriginalFileObject->Flags & FO_SYNCHRONOUS_IO &&
Irp->UserEvent != &OriginalFileObject->Event)
- {
- KeSetEvent(&OriginalFileObject->Event,0,FALSE);
- }
- }
-
- //Windows NT File System Internals, page 154
- if (OriginalFileObject)
- {
- // if the event is not the one in the file object, it needs
dereferenced
- if (Irp->UserEvent && Irp->UserEvent !=
&OriginalFileObject->Event)
- {
- ObDereferenceObject(Irp->UserEvent);
- }
-
- ObDereferenceObject(OriginalFileObject);
- }
-
- if (Irp->Overlay.AsynchronousParameters.UserApcRoutine != NULL)
- {
- PKNORMAL_ROUTINE UserApcRoutine;
- PVOID UserApcContext;
-
- DPRINT("Dispatching user APC\n");
-
- UserApcRoutine =
(PKNORMAL_ROUTINE)Irp->Overlay.AsynchronousParameters.UserApcRoutine;
- UserApcContext =
(PVOID)Irp->Overlay.AsynchronousParameters.UserApcContext;
-
- KeInitializeApc( &Irp->Tail.Apc,
- KeGetCurrentThread(),
- CurrentApcEnvironment,
- IoSecondStageCompletion_KernelApcRoutine,
- IoSecondStageCompletion_RundownApcRoutine,
- UserApcRoutine,
- UserMode,
- UserApcContext);
-
- KeInsertQueueApc( &Irp->Tail.Apc,
- Irp->UserIosb,
- NULL,
- 2);
-
- //NOTE: kernel (or rundown) routine frees the IRP
-
- return;
-
- }
-
- if (NULL != IoStack->FileObject
- && NULL != IoStack->FileObject->CompletionContext
- && (0 != (Irp->Flags & IRP_SYNCHRONOUS_API)
- || 0 == (IoStack->FileObject->Flags & FO_SYNCHRONOUS_IO)))
- {
- PFILE_OBJECT FileObject = IoStack->FileObject;
- IoSetIoCompletion(FileObject->CompletionContext->Port,
- FileObject->CompletionContext->Key,
-
Irp->Overlay.AsynchronousParameters.UserApcContext,
- Irp->IoStatus.Status,
- Irp->IoStatus.Information,
- FALSE);
- }
-
- IoFreeIrp(Irp);
+ KeInsertQueueApc(&Irp->Tail.Apc,
+ Irp->UserIosb,
+ NULL,
+ 2);
+ }
+ else if (FileObject && FileObject->CompletionContext)
+ {
+ /* Call the IO Completion Port if we have one, instead */
+ IoSetIoCompletion(FileObject->CompletionContext->Port,
+ FileObject->CompletionContext->Key,
+
Irp->Overlay.AsynchronousParameters.UserApcContext,
+ Irp->IoStatus.Status,
+ Irp->IoStatus.Information,
+ FALSE);
+ }
+ else
+ {
+ /* Don't have anything, free it */
+ IoFreeIrp(Irp);
+ }
+
+ /* Dereference the File Object */
+ if (FileObject) ObDereferenceObject(FileObject);
+ }
+ else
+ {
+ /* Remove the IRP from the list of Thread Pending IRPs */
+ RemoveEntryList(&Irp->ThreadListEntry);
+ InitializeListHead(&Irp->ThreadListEntry);
+
+ /* Signal the Events only if PendingReturned and we have a File
Object */
+ if (FileObject && Irp->PendingReturned)
+ {
+ /* Check for SYNC IRP */
+ if (Irp->Flags & IRP_SYNCHRONOUS_API)
+ {
+ /* Set the status in this case only */
+ *Irp->UserIosb = Irp->IoStatus;
+
+ /* Signal our event if we have one */
+ if (Irp->UserEvent)
+ {
+ KeSetEvent(Irp->UserEvent, 0, FALSE);
+ }
+ else
+ {
+ /* Signal the File's Event instead */
+ KeSetEvent(&FileObject->Event, 0, FALSE);
+ }
+ }
+ else
+ {
+ /* We'll report the Status to the File Object, not the
IRP */
+ FileObject->FinalStatus = Irp->IoStatus.Status;
+
+ /* Signal the File Object ONLY if this was Async */
+ KeSetEvent(&FileObject->Event, 0, FALSE);
+ }
+ }
+
+ /* Dereference the Event if it's an ASYNC IRP on a File Object
*/
+ if (Irp->UserEvent && !(Irp->Flags & IRP_SYNCHRONOUS_API)
&&
FileObject)
+ {
+ ObDereferenceObject(Irp->UserEvent);
+ }
+
+ /* Dereference the File Object */
+ if (FileObject) ObDereferenceObject(FileObject);
+
+ /* Free the IRP */
+ IoFreeIrp(Irp);
+ }
}
/* EOF */