[ekohl] 56861: [SERVICES] Correct comments and code error introduced in rev 56844. Patch by Hermes Belusca. I used a helper variable to determine whether or not ScmDeleteNamedPipeCriticalSection mu...

Author: ekohl
Date: Sun Jul  8 23:08:19 2012
New Revision: 56861

URL: http://svn.reactos.org/svn/reactos?rev=56861&view=rev
Log:
[SERVICES]
Correct comments and code error introduced in rev 56844.
Patch by Hermes Belusca.

I used a helper variable to determine whether or not ScmDeleteNamedPipeCriticalSection
must be called. Changing the order in which functions are called could cause a deadlock.

See issue #7172 for more details.

Modified:
    trunk/reactos/base/system/services/lock.c
    trunk/reactos/base/system/services/rpcserver.c
    trunk/reactos/base/system/services/services.c

Modified: trunk/reactos/base/system/services/lock.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/base/system/services/lock.…
==============================================================================
--- trunk/reactos/base/system/services/lock.c [iso-8859-1] (original)
+++ trunk/reactos/base/system/services/lock.c [iso-8859-1] Sun Jul  8 23:08:19 2012
@@ -22,9 +22,9 @@
 /* FUNCTIONS *****************************************************************/
 
 /*
- * TRUE if locked by the Service Control Manager, FALSE otherwise
+ * NOTE: IsServiceController is TRUE if locked by the
+ * Service Control Manager, and FALSE otherwise.
  */
-
 DWORD
 ScmAcquireServiceStartLock(IN BOOL IsServiceController,
                            OUT LPSC_RPC_LOCK lpLock)
@@ -146,7 +146,7 @@
         lpLockStatus->dwLockDuration = 0;
     }
 
-    /* Unlock the whole SC manager */
+    /* Unlock the service database */
     ScmUnlockDatabase();
 
     return;
@@ -178,7 +178,7 @@
         lpLockStatus->dwLockDuration = 0;
     }
 
-    /* Unlock the whole SC manager */
+    /* Unlock the service database */
     ScmUnlockDatabase();
 
     return;

Modified: trunk/reactos/base/system/services/rpcserver.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/base/system/services/rpcse…
==============================================================================
--- trunk/reactos/base/system/services/rpcserver.c [iso-8859-1] (original)
+++ trunk/reactos/base/system/services/rpcserver.c [iso-8859-1] Sun Jul  8 23:08:19 2012
@@ -2877,7 +2877,7 @@
         return ERROR_ACCESS_DENIED;
     }
 
-    /* HACK: we need to compute instead the real length of the owner name */
+    /* FIXME: we need to compute instead the real length of the owner name */
     dwRequiredSize = sizeof(QUERY_SERVICE_LOCK_STATUSW) + sizeof(WCHAR);
     *pcbBytesNeeded = dwRequiredSize;
 
@@ -4243,16 +4243,13 @@
         }
     }
 
-    /* Start the service */
-    dwError = ScmStartService(lpService, argc, lpVector);
-
     /* Acquire the service start lock until the service has been started */
     dwError = ScmAcquireServiceStartLock(TRUE, &Lock);
     if (dwError != ERROR_SUCCESS)
         goto done;
 
-     /* Start the service */
-     dwError = ScmStartService(lpService, argc, lpVector);
+    /* Start the service */
+    dwError = ScmStartService(lpService, argc, lpVector);
 
      /* Release the service start lock */
      ScmReleaseServiceStartLock(&Lock);

Modified: trunk/reactos/base/system/services/services.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/base/system/services/servi…
==============================================================================
--- trunk/reactos/base/system/services/services.c [iso-8859-1] (original)
+++ trunk/reactos/base/system/services/services.c [iso-8859-1] Sun Jul  8 23:08:19 2012
@@ -378,6 +378,7 @@
 {
     HANDLE hScmStartEvent = NULL;
     SC_RPC_LOCK Lock = NULL;
+    BOOL bDeleteCriticalSection = FALSE;
     DWORD dwError;
 
     DPRINT("SERVICES: Service Control Manager\n");
@@ -442,6 +443,7 @@
     AcquireLoadDriverPrivilege();
 
     ScmInitNamedPipeCriticalSection();
+    bDeleteCriticalSection = TRUE;
 
     /* Acquire the service start lock until autostart services have been started */
     dwError = ScmAcquireServiceStartLock(TRUE, &Lock);
@@ -465,7 +467,8 @@
     WaitForSingleObject(hScmShutdownEvent, INFINITE);
 
 done:
-    ScmDeleteNamedPipeCriticalSection();
+    if (bDeleteCriticalSection == TRUE)
+        ScmDeleteNamedPipeCriticalSection();
 
     /* Close the shutdown event */
     if (hScmShutdownEvent != NULL)