[ros-diffs] [hbelusca] 57866: [CONSRV] Put the "Move to the next entry" code just after retrieving list record, because it is less bug-prone. Indeed correct a bug in CsrDestroyProcess where we were doing an in...

Author: hbelusca
Date: Tue Dec 11 01:25:57 2012
New Revision: 57866

URL: http://svn.reactos.org/svn/reactos?rev=57866&view=rev
Log:
[CONSRV]
Put the "Move to the next entry" code just after retrieving list record, because
it is less bug-prone.
Indeed correct a bug in CsrDestroyProcess where we were doing an infinite loop when trying
to notify existing CSR waits, because NextEntry was set to the
CsrProcess->ThreadList.Flink element (head) of the list.

Modified:
    branches/ros-csrss/subsystems/win32/csrsrv/procsup.c
    branches/ros-csrss/subsystems/win32/csrsrv/thredsup.c

Modified: branches/ros-csrss/subsystems/win32/csrsrv/procsup.c
URL:
http://svn.reactos.org/svn/reactos/branches/ros-csrss/subsystems/win32/csrs…
==============================================================================
--- branches/ros-csrss/subsystems/win32/csrsrv/procsup.c [iso-8859-1] (original)
+++ branches/ros-csrss/subsystems/win32/csrsrv/procsup.c [iso-8859-1] Tue Dec 11 01:25:57
2012
@@ -854,10 +854,13 @@
         /* Get the current thread entry */
         CsrThread = CONTAINING_RECORD(NextEntry, CSR_THREAD, Link);
 
+        /* Move to the next entry */
+        NextEntry = NextEntry->Flink;
+
         /* Make sure the thread isn't already dead */
         if (CsrThread->Flags & CsrThreadTerminated)
         {
-            NextEntry = NextEntry->Flink;
+            /* Go the the next thread */
             continue;
         }
 
@@ -884,7 +887,6 @@
 
         /* Dereference the thread */
         CsrLockedDereferenceThread(CsrThread);
-        NextEntry = CsrProcess->ThreadList.Flink;
     }
 
     /* Release the Process Lock and return success */
@@ -1100,7 +1102,7 @@
             break;
         }
 
-        /* Next entry */
+        /* Move to the next entry */
         NextEntry = NextEntry->Flink;
     } while (NextEntry != &CsrRootProcess->ListLink);
 
@@ -1279,12 +1281,12 @@
         /* Get the Process */
         CsrProcess = CONTAINING_RECORD(NextEntry, CSR_PROCESS, ListLink);
 
-        /* Remove the skip flag, set shutdown flags to 0*/
+        /* Move to the next entry */
+        NextEntry = NextEntry->Flink;
+
+        /* Remove the skip flag, set shutdown flags to 0 */
         CsrProcess->Flags &= ~CsrProcessSkipShutdown;
         CsrProcess->ShutdownFlags = 0;
-
-        /* Move to the next */
-        NextEntry = NextEntry->Flink;
     }
 
     /* Set shudown Priority */
@@ -1394,12 +1396,12 @@
         /* Get the Process */
         CsrProcess = CONTAINING_RECORD(NextEntry, CSR_PROCESS, ListLink);
 
-        /* Remove the skip flag, set shutdown flags to 0*/
+        /* Move to the next entry */
+        NextEntry = NextEntry->Flink;
+
+        /* Remove the skip flag, set shutdown flags to 0 */
         CsrProcess->Flags &= ~CsrProcessSkipShutdown;
         CsrProcess->ShutdownFlags = 0;
-
-        /* Move to the next */
-        NextEntry = NextEntry->Flink;
     }
     
     /* Set shudown Priority */

Modified: branches/ros-csrss/subsystems/win32/csrsrv/thredsup.c
URL:
http://svn.reactos.org/svn/reactos/branches/ros-csrss/subsystems/win32/csrs…
==============================================================================
--- branches/ros-csrss/subsystems/win32/csrsrv/thredsup.c [iso-8859-1] (original)
+++ branches/ros-csrss/subsystems/win32/csrsrv/thredsup.c [iso-8859-1] Tue Dec 11 01:25:57
2012
@@ -201,6 +201,9 @@
         /* Get the thread */
         FoundThread = CONTAINING_RECORD(NextEntry, CSR_THREAD, HashLinks);
 
+        /* Move to the next entry */
+        NextEntry = NextEntry->Flink;
+
         /* Compare the CID */
         // FIXME: if (*(PULONGLONG)&FoundThread->ClientId ==
*(PULONGLONG)ClientId)
         if (FoundThread->ClientId.UniqueThread == ClientId->UniqueThread)
@@ -212,9 +215,6 @@
 //            DPRINT1("Found: %p %p\n", FoundThread, FoundThread->Process);
             return FoundThread;
         }
-
-        /* Next */
-        NextEntry = NextEntry->Flink;
     }
 
     /* Nothing found */
@@ -267,7 +267,7 @@
         /* Check for TID Match */
         if (FoundThread->ClientId.UniqueThread == Cid->UniqueThread) break;
 
-        /* Next entry */
+        /* Move to the next entry */
         NextEntry = NextEntry->Flink;
     }
 
@@ -970,6 +970,9 @@
         /* Get the Process */
         CurrentThread = CONTAINING_RECORD(NextEntry, CSR_THREAD, HashLinks);
 
+        /* Move to the next entry */
+        NextEntry = NextEntry->Flink;
+
         /* Check for PID Match */
         if ((CurrentThread->ClientId.UniqueThread == Tid) &&
             !(CurrentThread->Flags & CsrThreadTerminated))
@@ -977,9 +980,6 @@
             /* Get out of here */
             break;
         }
-
-        /* Next entry */
-        NextEntry = NextEntry->Flink;
     }
 
     /* Nothing found if we got back to the list */