24 Feb
2018
24 Feb
'18
10:31 a.m.
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=1dfbed9c3dbd135c7bf51…
commit 1dfbed9c3dbd135c7bf51090b9cadde6d2642340
Author: Eric Kohl <eric.kohl(a)reactos.org>
AuthorDate: Sat Feb 24 11:14:05 2018 +0100
Commit: Eric Kohl <eric.kohl(a)reactos.org>
CommitDate: Sat Feb 24 11:14:05 2018 +0100
[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 | 105 +++++++++++++++++++------------------
base/system/services/rpcserver.c | 28 +++++++++-
base/system/services/services.h | 5 +-
dll/win32/advapi32/service/sctrl.c | 70 +++++++++++++------------
4 files changed, 121 insertions(+), 87 deletions(-)
diff --git a/base/system/services/database.c b/base/system/services/database.c
index d31d353a87..d28a98c082 100644
--- a/base/system/services/database.c
+++ b/base/system/services/database.c
@@ -432,41 +432,34 @@ done:
}
-static VOID
-ScmDereferenceServiceImage(PSERVICE_IMAGE pServiceImage)
+VOID
+ScmRemoveServiceImage(PSERVICE_IMAGE pServiceImage)
{
- DPRINT1("ScmDereferenceServiceImage() called\n");
-
- pServiceImage->dwImageRunCount--;
-
- if (pServiceImage->dwImageRunCount == 0)
- {
- DPRINT1("dwImageRunCount == 0\n");
+ DPRINT1("ScmRemoveServiceImage() called\n");
- /* FIXME: Terminate the process */
+ /* FIXME: Terminate the process */
- /* Remove the service image from the list */
- RemoveEntryList(&pServiceImage->ImageListEntry);
+ /* Remove the service image from the list */
+ RemoveEntryList(&pServiceImage->ImageListEntry);
- /* Close the process handle */
- if (pServiceImage->hProcess != INVALID_HANDLE_VALUE)
- CloseHandle(pServiceImage->hProcess);
+ /* Close the process handle */
+ if (pServiceImage->hProcess != INVALID_HANDLE_VALUE)
+ CloseHandle(pServiceImage->hProcess);
- /* Close the control pipe */
- if (pServiceImage->hControlPipe != INVALID_HANDLE_VALUE)
- CloseHandle(pServiceImage->hControlPipe);
+ /* Close the control pipe */
+ if (pServiceImage->hControlPipe != INVALID_HANDLE_VALUE)
+ CloseHandle(pServiceImage->hControlPipe);
- /* Unload the user profile */
- if (pServiceImage->hProfile != NULL)
- UnloadUserProfile(pServiceImage->hToken, pServiceImage->hProfile);
+ /* Unload the user profile */
+ if (pServiceImage->hProfile != NULL)
+ UnloadUserProfile(pServiceImage->hToken, pServiceImage->hProfile);
- /* Close the logon token */
- if (pServiceImage->hToken != NULL)
- CloseHandle(pServiceImage->hToken);
+ /* Close the logon token */
+ if (pServiceImage->hToken != NULL)
+ CloseHandle(pServiceImage->hToken);
- /* Release the service image */
- HeapFree(GetProcessHeap(), 0, pServiceImage);
- }
+ /* Release the service image */
+ HeapFree(GetProcessHeap(), 0, pServiceImage);
}
@@ -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,8 +1827,12 @@ ScmLoadService(PSERVICE Service,
}
else
{
- ScmDereferenceServiceImage(Service->lpImage);
- Service->lpImage = NULL;
+ Service->lpImage->dwImageRunCount--;
+ if (Service->lpImage->dwImageRunCount == 0)
+ {
+ ScmRemoveServiceImage(Service->lpImage);
+ Service->lpImage = NULL;
+ }
}
}
}
@@ -2168,8 +2168,11 @@ ScmAutoShutdownServices(VOID)
CurrentService->Status.dwCurrentState == SERVICE_START_PENDING)
{
/* shutdown service */
- DPRINT("Shutdown service: %S\n",
CurrentService->szServiceName);
- ScmControlService(CurrentService, SERVICE_CONTROL_SHUTDOWN);
+ DPRINT("Shutdown service: %S\n",
CurrentService->lpServiceName);
+ ScmControlService(CurrentService->lpImage->hControlPipe,
+ CurrentService->lpServiceName,
+ (SERVICE_STATUS_HANDLE)CurrentService,
+ SERVICE_CONTROL_SHUTDOWN);
}
ServiceEntry = ServiceEntry->Flink;
diff --git a/base/system/services/rpcserver.c b/base/system/services/rpcserver.c
index a4718aa15f..0a518d06ad 100644
--- a/base/system/services/rpcserver.c
+++ b/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");
diff --git a/base/system/services/services.h b/base/system/services/services.h
index 0f03849967..3592ba2560 100644
--- a/base/system/services/services.h
+++ b/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);
diff --git a/dll/win32/advapi32/service/sctrl.c b/dll/win32/advapi32/service/sctrl.c
index 4a0ae5665d..fe2469732f 100644
--- a/dll/win32/advapi32/service/sctrl.c
+++ b/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,39 +572,44 @@ ScServiceDispatcher(HANDLE hPipe,
lpServiceName = (LPWSTR)((PBYTE)ControlPacket +
ControlPacket->dwServiceNameOffset);
TRACE("Service: %S\n", lpServiceName);
- if (ControlPacket->dwControl == SERVICE_CONTROL_START_OWN)
- lpActiveServices[0].bOwnProcess = TRUE;
-
- lpService = ScLookupServiceByServiceName(lpServiceName);
- if (lpService != NULL)
+ if (lpServiceName[0] == UNICODE_NULL)
{
- /* Execute command */
- switch (ControlPacket->dwControl)
- {
- case SERVICE_CONTROL_START_SHARE:
- 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:
- TRACE("Command %lu received",
ControlPacket->dwControl);
- dwError = ScControlService(lpService, ControlPacket);
- break;
- }
+ ERR("Stop dispatcher thread\n");
+ bRunning = FALSE;
+ dwError = ERROR_SUCCESS;
}
else
{
- dwError = ERROR_SERVICE_DOES_NOT_EXIST;
+ if (ControlPacket->dwControl == SERVICE_CONTROL_START_OWN)
+ lpActiveServices[0].bOwnProcess = TRUE;
+
+ lpService = ScLookupServiceByServiceName(lpServiceName);
+ if (lpService != NULL)
+ {
+ /* Execute command */
+ switch (ControlPacket->dwControl)
+ {
+ case SERVICE_CONTROL_START_SHARE:
+ case SERVICE_CONTROL_START_OWN:
+ TRACE("Start command - received
SERVICE_CONTROL_START\n");
+ dwError = ScStartService(lpService, ControlPacket);
+ break;
+
+ case SERVICE_CONTROL_STOP:
+ TRACE("Stop command - received
SERVICE_CONTROL_STOP\n");
+ dwError = ScControlService(lpService, ControlPacket);
+ break;
+
+ default:
+ TRACE("Command %lu received",
ControlPacket->dwControl);
+ dwError = ScControlService(lpService, ControlPacket);
+ break;
+ }
+ }
+ else
+ {
+ 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;
2453
days inactive
2453
days old
0 comments
1 participants
participants (1)
-
Eric Kohl