[SERVICES][ADVAPI32] Fix the broken service stop sequence

services\database.c:
- Refactor ScmControlService() so that it can be used to send the dispatcher loop stop command.
- Separate the code to decrement the image run counter from the service image cleanup code.

services\rpcserver.c:
- RSetServiceStatus(): Stop the dispatcher loop when the image run counter is zero and remove the service image after that.

advapi32\service\sctrl.c:
- Do not terminate the service dispatcher loop when the last service is being stopped. Wait for an explicit dispatcher stop command (empty service name).

CORE-12413

base/system/services/database.c

@@ -432,16 +432,10 @@ done:
 }
 
 
-static VOID
+VOID
-ScmDereferenceServiceImage(PSERVICE_IMAGE pServiceImage)
+ScmRemoveServiceImage(PSERVICE_IMAGE pServiceImage)
 {
-    DPRINT1("ScmDereferenceServiceImage() called\n");
+    DPRINT1("ScmRemoveServiceImage() called\n");
-
-    pServiceImage->dwImageRunCount--;
-
-    if (pServiceImage->dwImageRunCount == 0)
-    {
-        DPRINT1("dwImageRunCount == 0\n");
 
     /* FIXME: Terminate the process */
 
@@ -467,7 +461,6 @@ ScmDereferenceServiceImage(PSERVICE_IMAGE pServiceImage)
     /* Release the service image */
     HeapFree(GetProcessHeap(), 0, pServiceImage);
 }
-}
 
 
 PSERVICE
@@ -618,7 +611,15 @@ ScmDeleteServiceRecord(PSERVICE lpService)
 
     /* Dereference the service image */
     if (lpService->lpImage)
-        ScmDereferenceServiceImage(lpService->lpImage);
+    {
+        lpService->lpImage->dwImageRunCount--;
+
+        if (lpService->lpImage->dwImageRunCount == 0)
+        {
+            ScmRemoveServiceImage(lpService->lpImage);
+            lpService->lpImage = NULL;
+        }
+    }
 
     /* Decrement the group reference counter */
     ScmSetServiceGroup(lpService, NULL);
@@ -1095,7 +1096,9 @@ ScmGetBootAndSystemDriverState(VOID)
 
 
 DWORD
-ScmControlService(PSERVICE Service,
+ScmControlService(HANDLE hControlPipe,
+                  PWSTR pServiceName,
+                  SERVICE_STATUS_HANDLE hServiceStatus,
                   DWORD dwControl)
 {
     PSCM_CONTROL_PACKET ControlPacket;
@@ -1116,7 +1119,7 @@ ScmControlService(PSERVICE Service,
 
     /* Calculate the total length of the start command line */
     PacketSize = sizeof(SCM_CONTROL_PACKET);
-    PacketSize += (DWORD)((wcslen(Service->lpServiceName) + 1) * sizeof(WCHAR));
+    PacketSize += (DWORD)((wcslen(pServiceName) + 1) * sizeof(WCHAR));
 
     ControlPacket = HeapAlloc(GetProcessHeap(),
                               HEAP_ZERO_MEMORY,
@@ -1129,17 +1132,17 @@ ScmControlService(PSERVICE Service,
 
     ControlPacket->dwSize = PacketSize;
     ControlPacket->dwControl = dwControl;
-    ControlPacket->hServiceStatus = (SERVICE_STATUS_HANDLE)Service;
+    ControlPacket->hServiceStatus = hServiceStatus;
 
     ControlPacket->dwServiceNameOffset = sizeof(SCM_CONTROL_PACKET);
 
     Ptr = (PWSTR)((PBYTE)ControlPacket + ControlPacket->dwServiceNameOffset);
-    wcscpy(Ptr, Service->lpServiceName);
+    wcscpy(Ptr, pServiceName);
 
     ControlPacket->dwArgumentsCount = 0;
     ControlPacket->dwArgumentsOffset = 0;
 
-    bResult = WriteFile(Service->lpImage->hControlPipe,
+    bResult = WriteFile(hControlPipe,
                         ControlPacket,
                         PacketSize,
                         &dwWriteCount,
@@ -1153,13 +1156,13 @@ ScmControlService(PSERVICE Service,
         {
             DPRINT("dwError: ERROR_IO_PENDING\n");
 
-            dwError = WaitForSingleObject(Service->lpImage->hControlPipe,
+            dwError = WaitForSingleObject(hControlPipe,
                                           PipeTimeout);
             DPRINT("WaitForSingleObject() returned %lu\n", dwError);
 
             if (dwError == WAIT_TIMEOUT)
             {
-                bResult = CancelIo(Service->lpImage->hControlPipe);
+                bResult = CancelIo(hControlPipe);
                 if (bResult == FALSE)
                 {
                     DPRINT1("CancelIo() failed (Error: %lu)\n", GetLastError());
@@ -1170,7 +1173,7 @@ ScmControlService(PSERVICE Service,
             }
             else if (dwError == WAIT_OBJECT_0)
             {
-                bResult = GetOverlappedResult(Service->lpImage->hControlPipe,
+                bResult = GetOverlappedResult(hControlPipe,
                                               &Overlapped,
                                               &dwWriteCount,
                                               TRUE);
@@ -1193,7 +1196,7 @@ ScmControlService(PSERVICE Service,
     /* Read the reply */
     Overlapped.hEvent = (HANDLE) NULL;
 
-    bResult = ReadFile(Service->lpImage->hControlPipe,
+    bResult = ReadFile(hControlPipe,
                        &ReplyPacket,
                        sizeof(SCM_REPLY_PACKET),
                        &dwReadCount,
@@ -1207,13 +1210,13 @@ ScmControlService(PSERVICE Service,
         {
             DPRINT("dwError: ERROR_IO_PENDING\n");
 
-            dwError = WaitForSingleObject(Service->lpImage->hControlPipe,
+            dwError = WaitForSingleObject(hControlPipe,
                                           PipeTimeout);
             DPRINT("WaitForSingleObject() returned %lu\n", dwError);
 
             if (dwError == WAIT_TIMEOUT)
             {
-                bResult = CancelIo(Service->lpImage->hControlPipe);
+                bResult = CancelIo(hControlPipe);
                 if (bResult == FALSE)
                 {
                     DPRINT1("CancelIo() failed (Error: %lu)\n", GetLastError());
@@ -1224,7 +1227,7 @@ ScmControlService(PSERVICE Service,
             }
             else if (dwError == WAIT_OBJECT_0)
             {
-                bResult = GetOverlappedResult(Service->lpImage->hControlPipe,
+                bResult = GetOverlappedResult(hControlPipe,
                                               &Overlapped,
                                               &dwReadCount,
                                               TRUE);
@@ -1255,13 +1258,6 @@ Done:
         dwError = ReplyPacket.dwError;
     }
 
-    if (dwError == ERROR_SUCCESS &&
-        dwControl == SERVICE_CONTROL_STOP)
-    {
-        ScmDereferenceServiceImage(Service->lpImage);
-        Service->lpImage = NULL;
-    }
-
     LeaveCriticalSection(&ControlServiceCriticalSection);
 
     DPRINT("ScmControlService() done\n");
@@ -1831,11 +1827,15 @@ ScmLoadService(PSERVICE Service,
             }
             else
             {
-                ScmDereferenceServiceImage(Service->lpImage);
+                Service->lpImage->dwImageRunCount--;
+                if (Service->lpImage->dwImageRunCount == 0)
+                {
+                    ScmRemoveServiceImage(Service->lpImage);
                     Service->lpImage = NULL;
                 }
             }
         }
+    }
 
     DPRINT("ScmLoadService() done (Error %lu)\n", dwError);
 
@@ -2168,8 +2168,11 @@ ScmAutoShutdownServices(VOID)
             CurrentService->Status.dwCurrentState == SERVICE_START_PENDING)
         {
             /* shutdown service */
-            DPRINT("Shutdown service: %S\n", CurrentService->szServiceName);
+            DPRINT("Shutdown service: %S\n", CurrentService->lpServiceName);
-            ScmControlService(CurrentService, SERVICE_CONTROL_SHUTDOWN);
+            ScmControlService(CurrentService->lpImage->hControlPipe,
+                              CurrentService->lpServiceName,
+                              (SERVICE_STATUS_HANDLE)CurrentService,
+                              SERVICE_CONTROL_SHUTDOWN);
         }
 
         ServiceEntry = ServiceEntry->Flink;

base/system/services/rpcserver.c

@@ -1228,7 +1228,9 @@ RControlService(
         }
 
         /* Send control code to the service */
-        dwError = ScmControlService(lpService,
+        dwError = ScmControlService(lpService->lpImage->hControlPipe,
+                                    lpService->lpServiceName,
+                                    (SERVICE_STATUS_HANDLE)lpService,
                                     dwControl);
 
         /* Return service status information */
@@ -1725,6 +1727,28 @@ RSetServiceStatus(
     /* Restore the previous service type */
     lpService->Status.dwServiceType = dwPreviousType;
 
+    /* Handle a stopped service */
+    if ((lpServiceStatus->dwServiceType & SERVICE_WIN32) &&
+        (lpServiceStatus->dwCurrentState == SERVICE_STOPPED))
+    {
+        /* Decrement the image run counter */
+        lpService->lpImage->dwImageRunCount--;
+
+        /* If we just stopped the last running service... */
+        if (lpService->lpImage->dwImageRunCount == 0)
+        {
+            /* Stop the dispatcher thread */
+            ScmControlService(lpService->lpImage->hControlPipe,
+                              L"",
+                              (SERVICE_STATUS_HANDLE)lpService,
+                              SERVICE_CONTROL_STOP);
+
+            /* Remove the service image */
+            ScmRemoveServiceImage(lpService->lpImage);
+            lpService->lpImage = NULL;
+        }
+    }
+
     /* Unlock the service database */
     ScmUnlockDatabase();
 
@@ -1773,6 +1797,8 @@ RSetServiceStatus(
                     lpLogStrings);
     }
 
+
+
     DPRINT("Set %S to %lu\n", lpService->lpDisplayName, lpService->Status.dwCurrentState);
     DPRINT("RSetServiceStatus() done\n");
 

base/system/services/services.h

@@ -164,6 +164,7 @@ DWORD ScmStartService(PSERVICE Service,
                       DWORD argc,
                       LPWSTR *argv);
 
+VOID ScmRemoveServiceImage(PSERVICE_IMAGE pServiceImage);
 PSERVICE ScmGetServiceEntryByName(LPCWSTR lpServiceName);
 PSERVICE ScmGetServiceEntryByDisplayName(LPCWSTR lpDisplayName);
 PSERVICE ScmGetServiceEntryByResumeCount(DWORD dwResumeCount);
@@ -174,7 +175,9 @@ DWORD ScmCreateNewServiceRecord(LPCWSTR lpServiceName,
 VOID ScmDeleteServiceRecord(PSERVICE lpService);
 DWORD ScmMarkServiceForDelete(PSERVICE pService);
 
-DWORD ScmControlService(PSERVICE Service,
+DWORD ScmControlService(HANDLE hControlPipe,
+                        PWSTR pServiceName,
+                        SERVICE_STATUS_HANDLE hServiceStatus,
                         DWORD dwControl);
 
 BOOL ScmLockDatabaseExclusive(VOID);

dll/win32/advapi32/service/sctrl.c

@@ -544,7 +544,7 @@ ScServiceDispatcher(HANDLE hPipe,
 {
     DWORD Count;
     BOOL bResult;
-    DWORD dwRunningServices = 0;
+    BOOL bRunning = TRUE;
     LPWSTR lpServiceName;
     PACTIVE_SERVICE lpService;
     SCM_REPLY_PACKET ReplyPacket;
@@ -555,7 +555,7 @@ ScServiceDispatcher(HANDLE hPipe,
     if (ControlPacket == NULL || dwBufferSize < sizeof(SCM_CONTROL_PACKET))
         return FALSE;
 
-    while (TRUE)
+    while (bRunning)
     {
         /* Read command from the control pipe */
         bResult = ReadFile(hPipe,
@@ -572,6 +572,14 @@ ScServiceDispatcher(HANDLE hPipe,
         lpServiceName = (LPWSTR)((PBYTE)ControlPacket + ControlPacket->dwServiceNameOffset);
         TRACE("Service: %S\n", lpServiceName);
 
+        if (lpServiceName[0] == UNICODE_NULL)
+        {
+            ERR("Stop dispatcher thread\n");
+            bRunning = FALSE;
+            dwError = ERROR_SUCCESS;
+        }
+        else
+        {
             if (ControlPacket->dwControl == SERVICE_CONTROL_START_OWN)
                 lpActiveServices[0].bOwnProcess = TRUE;
 
@@ -585,15 +593,11 @@ ScServiceDispatcher(HANDLE hPipe,
                     case SERVICE_CONTROL_START_OWN:
                         TRACE("Start command - received SERVICE_CONTROL_START\n");
                         dwError = ScStartService(lpService, ControlPacket);
-                    if (dwError == ERROR_SUCCESS)
-                        dwRunningServices++;
                         break;
 
                     case SERVICE_CONTROL_STOP:
                         TRACE("Stop command - received SERVICE_CONTROL_STOP\n");
                         dwError = ScControlService(lpService, ControlPacket);
-                    if (dwError == ERROR_SUCCESS)
-                        dwRunningServices--;
                         break;
 
                     default:
@@ -606,6 +610,7 @@ ScServiceDispatcher(HANDLE hPipe,
             {
                 dwError = ERROR_SERVICE_DOES_NOT_EXIST;
             }
+        }
 
         ReplyPacket.dwError = dwError;
 
@@ -620,9 +625,6 @@ ScServiceDispatcher(HANDLE hPipe,
             ERR("Pipe write failed (Error: %lu)\n", GetLastError());
             return FALSE;
         }
-
-        if (dwRunningServices == 0)
-            break;
     }
 
     return TRUE;