fixed a few race conditions during thread/process termination leading to dead-locks
[reactos.git] / reactos / ntoskrnl / ps / kill.c
index f191cd0..cda7b57 100644 (file)
@@ -101,13 +101,11 @@ PspTerminateProcessThreads(PEPROCESS Process,
         
             /* Make sure it didn't already terminate */
             if (!Thread->HasTerminated) {
-        
+
                 Thread->HasTerminated = TRUE;
+                
                 /* Terminate it by APC */
                 PspTerminateThreadByPointer(Thread, ExitStatus);
-                
-                /* Unsuspend it */
-                KeForceResumeThread(&Thread->Tcb);
             }
         }
     }
@@ -145,7 +143,7 @@ PspDeleteThread(PVOID ObjectBody)
     PETHREAD Thread = (PETHREAD)ObjectBody;
     PEPROCESS Process = Thread->ThreadsProcess;
 
-    DPRINT("PiDeleteThread(ObjectBody %x)\n",ObjectBody);
+    DPRINT("PiDeleteThread(ObjectBody 0x%x, process 0x%x)\n",ObjectBody, Thread->ThreadsProcess);
 
     /* Deassociate the Process */
     Thread->ThreadsProcess = NULL;
@@ -181,12 +179,15 @@ PspExitThread(NTSTATUS ExitStatus)
     PVOID TebBlock;
     PTERMINATION_PORT TerminationPort;
 
-    DPRINT("PsTerminateCurrentThread(ExitStatus %x)\n", ExitStatus);
+    DPRINT("PspExitThread(ExitStatus %x), Current: 0x%x\n", ExitStatus, PsGetCurrentThread());
 
     /* Get the Current Thread and Process */
     CurrentThread = PsGetCurrentThread();
-    CurrentThread->HasTerminated = TRUE;
     CurrentProcess = CurrentThread->ThreadsProcess;
+    
+    /* Set the Exit Status and Exit Time */
+    CurrentThread->ExitStatus = ExitStatus;
+    KeQuerySystemTime(&CurrentThread->ExitTime);
 
     /* Can't terminate a thread if it attached another process */
     if (KeIsAttachedProcess()) {
@@ -200,11 +201,14 @@ PspExitThread(NTSTATUS ExitStatus)
     /* Lower to Passive Level */
     KeLowerIrql(PASSIVE_LEVEL);
 
-    /* Run Thread Notify Routines before we desintegrate the thread */
-    PspRunCreateThreadNotifyRoutines(CurrentThread, FALSE);
-
     /* Lock the Process before we modify its thread entries */
     PsLockProcess(CurrentProcess, FALSE);
+    
+    /* wake up the thread so we don't deadlock on PsLockProcess */
+    KeForceResumeThread(&CurrentThread->Tcb);
+    
+    /* Run Thread Notify Routines before we desintegrate the thread */
+    PspRunCreateThreadNotifyRoutines(CurrentThread, FALSE);
 
     /* Remove the thread from the thread list of its process */
     RemoveEntryList(&CurrentThread->ThreadListEntry);
@@ -213,8 +217,13 @@ PspExitThread(NTSTATUS ExitStatus)
     /* Set the last Thread Exit Status */
     CurrentProcess->LastThreadExitStatus = ExitStatus;
     
-    /* Unlock the Process */
-    PsUnlockProcess(CurrentProcess);
+    if (Last) {
+
+       /* Save the Exit Time if not already done by NtTerminateProcess. This
+          happens when the last thread just terminates without explicitly
+          terminating the process. */
+       CurrentProcess->ExitTime = CurrentThread->ExitTime;
+    }
     
     /* Check if the process has a debug port */
     if (CurrentProcess->DebugPort) {
@@ -243,58 +252,55 @@ PspExitThread(NTSTATUS ExitStatus)
     PsTerminateWin32Thread(CurrentThread);
     if (Last) PsTerminateWin32Process(CurrentProcess);
    
-    /* Cancel I/O for the thread. */
-    IoCancelThreadIo(CurrentThread);
-   
-    /* Rundown Timers */
-    ExTimerRundown();
-    KeCancelTimer(&CurrentThread->Tcb.Timer);
-   
     /* Rundown Registry Notifications. TODO (refers to NtChangeNotify, not Cm callbacks) */
     //CmNotifyRunDown(CurrentThread);
-   
-    /* Rundown Mutexes */
-    KeRundownThread();
-     
+    
     /* Free the TEB */
     if(CurrentThread->Tcb.Teb) {
-        
+
         DPRINT("Decommit teb at %p\n", CurrentThread->Tcb.Teb);
-        ExAcquireFastMutex(&CurrentProcess->TebLock);
         TebBlock = MM_ROUND_DOWN(CurrentThread->Tcb.Teb, MM_VIRTMEM_GRANULARITY);
-     
+
         ZwFreeVirtualMemory(NtCurrentProcess(),
                            (PVOID *)&CurrentThread->Tcb.Teb,
                            &Length,
                            MEM_DECOMMIT);
-        
+
         DPRINT("teb %p, TebBlock %p\n", CurrentThread->Tcb.Teb, TebBlock);
-     
+
         if (TebBlock != CurrentProcess->TebBlock ||
             CurrentProcess->TebBlock == CurrentProcess->TebLastAllocated) {
-            
+
             MmLockAddressSpace(&CurrentProcess->AddressSpace);
             MmReleaseMemoryAreaIfDecommitted(CurrentProcess, &CurrentProcess->AddressSpace, TebBlock);
             MmUnlockAddressSpace(&CurrentProcess->AddressSpace);
         }
-    
+
         CurrentThread->Tcb.Teb = NULL;
-        ExReleaseFastMutex(&CurrentProcess->TebLock);
     }
    
-    /* Set the Exit Status and Exit Time */
-    CurrentThread->ExitStatus = ExitStatus;
-    KeQuerySystemTime((PLARGE_INTEGER)&CurrentThread->ExitTime);
-       
+    /* The last Thread shuts down the Process */
+    if (Last) PspExitProcess(CurrentProcess);
+    
+    /* Unlock the Process */
+    PsUnlockProcess(CurrentProcess);
+    
+    /* Cancel I/O for the thread. */
+    IoCancelThreadIo(CurrentThread);
+    
+    /* Rundown Timers */
+    ExTimerRundown();
+    KeCancelTimer(&CurrentThread->Tcb.Timer);
+    
     /* If the Processor Control Block's NpxThread points to the current thread
      * unset it.
      */
     InterlockedCompareExchangePointer(&KeGetCurrentPrcb()->NpxThread,
                                       NULL,
                                       (PKPROCESS)CurrentThread);
-   
-    /* The last Thread shuts down the Process */
-    if (Last) PspExitProcess(CurrentProcess);
+    
+    /* Rundown Mutexes */
+    KeRundownThread();
 
     /* Terminate the Thread from the Scheduler */
     KeTerminateThread(0);
@@ -312,11 +318,16 @@ PsExitSpecialApc(PKAPC Apc,
 {
     NTSTATUS ExitStatus = (NTSTATUS)Apc->NormalContext;
     
+    DPRINT("PsExitSpecialApc called: 0x%x (proc: 0x%x)\n", PsGetCurrentThread(), PsGetCurrentProcess());
+    
     /* Free the APC */
     ExFreePool(Apc);
     
     /* Terminate the Thread */
     PspExitThread(ExitStatus);
+    
+    /* we should never reach this point! */
+    KEBUGCHECK(0);
 }
 
 VOID 
@@ -330,6 +341,9 @@ PspExitNormalApc(PVOID NormalContext,
      */
     DPRINT1("APC2\n");
     PspExitThread((NTSTATUS)NormalContext);
+    
+    /* we should never reach this point! */
+    KEBUGCHECK(0);
 }
 
 /*
@@ -347,9 +361,12 @@ PspTerminateThreadByPointer(PETHREAD Thread,
     
     /* Check if we are already in the right context */
     if (PsGetCurrentThread() == Thread) {
-    
+
         /* Directly terminate the thread */
         PspExitThread(ExitStatus);
+
+        /* we should never reach this point! */
+        KEBUGCHECK(0);
     }
     
     /* Allocate the APC */
@@ -379,7 +396,7 @@ NTSTATUS
 STDCALL
 PspExitProcess(PEPROCESS Process)
 {
-    DPRINT("PspExitProcess\n");
+    DPRINT("PspExitProcess 0x%x\n", Process);
            
     PspRunCreateProcessNotifyRoutines(Process, FALSE);
            
@@ -388,12 +405,12 @@ PspExitProcess(PEPROCESS Process)
     RemoveEntryList(&Process->ProcessListEntry);
     ExReleaseFastMutex(&PspActiveProcessMutex);
     
+    /* close all handles associated with our process, this needs to be done
+       when the last thread still runs */
     ObKillProcess(Process);
-    KeSetProcess(&Process->Pcb, IO_NO_INCREMENT);
-
-    /* NOTE: This dereference corresponds to reference in NtTerminateProcess. */
-    ObDereferenceObject(Process);
 
+    KeSetProcess(&Process->Pcb, IO_NO_INCREMENT);
+    
     return(STATUS_SUCCESS);
 }
 
@@ -404,58 +421,79 @@ NtTerminateProcess(IN HANDLE ProcessHandle  OPTIONAL,
 {
     NTSTATUS Status;
     PEPROCESS Process;
+    PETHREAD CurrentThread;
+    BOOLEAN KillByHandle;
    
     PAGED_CODE();
    
     DPRINT("NtTerminateProcess(ProcessHandle %x, ExitStatus %x)\n",
             ProcessHandle, ExitStatus);
     
+    KillByHandle = (ProcessHandle != NULL);
+
     /* Get the Process Object */
-    Status = ObReferenceObjectByHandle(ProcessHandle,
+    Status = ObReferenceObjectByHandle((KillByHandle ? ProcessHandle : NtCurrentProcess()),
                                        PROCESS_TERMINATE,
                                        PsProcessType,
                                        KeGetPreviousMode(),
                                        (PVOID*)&Process,
                                        NULL);
     if (!NT_SUCCESS(Status)) {
-    
+
         DPRINT1("Invalid handle to Process\n");
         return(Status);
     }
     
+    CurrentThread = PsGetCurrentThread();
+    
     PsLockProcess(Process, FALSE);
-
-    if(Process->ExitTime.QuadPart) {
-        
-        DPRINT1("Process has an exit time!\n");
-        KeLeaveCriticalRegion();
-        return STATUS_PROCESS_IS_TERMINATING;
+    
+    if(Process->ExitTime.QuadPart != 0)
+    {
+      PsUnlockProcess(Process);
+      return STATUS_PROCESS_IS_TERMINATING;
     }
     
     /* Terminate all the Process's Threads */
     PspTerminateProcessThreads(Process, ExitStatus);
+    
+    /* only kill the calling thread if it either passed a process handle or
+       NtCurrentProcess() */
+    if (KillByHandle) {
 
-    /* Save the Exit Time */
-    KeQuerySystemTime(&Process->ExitTime);
+        /* set the exit time as we're about to release the process lock before
+           we kill ourselves to prevent threads outside of our process trying
+           to kill us */
+        KeQuerySystemTime(&Process->ExitTime);
 
-    PsUnlockProcess(Process);
-            
-    /* Only master thread remains... kill it off */
-    if (PsGetCurrentThread()->ThreadsProcess == Process) {
-        
-        /*
-         * NOTE: Dereferencing of the Process structure takes place in
-         * PspExitProcess. If we would do it here the Win32 Process
-         * information would be destroyed before the Win32 Destroy
-         * thread/process callback is called.
-         */
-        
-        PspExitThread(ExitStatus);
-        return(STATUS_SUCCESS);
+        /* Only master thread remains... kill it off */
+        if (CurrentThread->ThreadsProcess == Process) {
+
+            /* mark our thread as terminating so attempts to terminate it, when
+               unlocking the process, fail */
+            CurrentThread->HasTerminated = TRUE;
+
+            PsUnlockProcess(Process);
+
+            /* we can safely dereference the process because the current thread
+               holds a reference to it until it gets reaped */
+            ObDereferenceObject(Process);
+
+            /* now the other threads get a chance to terminate, we don't wait but
+               just kill ourselves right now. The process will be run down when the
+               last thread terminates */
+
+            PspExitThread(ExitStatus);
+
+            /* we should never reach this point! */
+            KEBUGCHECK(0);
+        }
     }
-    
-    /* If we took this path instead, then do the same as above */ 
+
+    /* unlock and dereference the process so the threads can kill themselves */
+    PsUnlockProcess(Process);
     ObDereferenceObject(Process);
+    
     return(STATUS_SUCCESS);
 }
 
@@ -476,7 +514,7 @@ NtTerminateThread(IN HANDLE ThreadHandle,
                                        KeGetPreviousMode(),
                                        (PVOID*)&Thread,
                                        NULL);
-    if (Status != STATUS_SUCCESS) {
+    if (!NT_SUCCESS(Status)) {
         
         DPRINT1("Could not reference thread object\n");
         return(Status);
@@ -493,26 +531,39 @@ NtTerminateThread(IN HANDLE ThreadHandle,
     /* Check to see if we're running in the same thread */
     if (Thread != PsGetCurrentThread())  {
                  
-         /* This isn't our thread, check if it's terminated already */
+        /* we need to lock the process to make sure it's not already terminating */
+        PsLockProcess(Thread->ThreadsProcess, FALSE);
+        
+        /* This isn't our thread, terminate it if not already done */
         if (!Thread->HasTerminated) {
          
+             Thread->HasTerminated = TRUE;
+             
              /* Terminate it */
              PspTerminateThreadByPointer(Thread, ExitStatus);
-             
-             /* Resume it */
-             KeForceResumeThread(&Thread->Tcb);
-         }
+        }
+        
+        PsUnlockProcess(Thread->ThreadsProcess);
+        
+        /* Dereference the Thread and return */
+        ObDereferenceObject(Thread);
         
     } else {
+
+        Thread->HasTerminated = TRUE;
+        
+        /* it's safe to dereference thread, there's at least the keep-alive
+           reference which will be removed by the thread reaper causing the
+           thread to be finally destroyed */
         ObDereferenceObject(Thread);
             
         /* Terminate him, he's ours */
         PspExitThread(ExitStatus);
-        /* We do never reach this point */        
+
+        /* We do never reach this point */
+        KEBUGCHECK(0);
     }
     
-    /* Dereference the Thread and return */
-    ObDereferenceObject(Thread);
     return(STATUS_SUCCESS);
 }
 
@@ -534,6 +585,10 @@ PsTerminateSystemThread(NTSTATUS ExitStatus)
         
     /* Terminate it for real */
     PspExitThread(ExitStatus);
+    
+    /* we should never reach this point! */
+    KEBUGCHECK(0);
+    
     return(STATUS_SUCCESS);
 }