Re: [ros-dev] [ros-diffs] [ekohl] 45640: Partially revert patches 45626 and 45633. Several services do not report their status to the service manager properly. Therefore we must not use any code that relies on service status information as part of the setup and boo

[Ged Murphy]

Are these our services and if so do you have a note of which ones?


-----Original Message-----
From: ros-diffs-bounces(a)reactos.org [mailto:[e-mail]] On Behalf Of
ekohl(a)svn.reactos.org
Sent: 20 February 2010 23:11
To: ros-diffs(a)reactos.org
Subject: [ros-diffs] [ekohl] 45640: Partially revert patches 45626 and 45633. Several
services do not report their status to the service manager properly. Therefore we must not
use any code that relies on service status information as part of the setup and boot pr

Author: ekohl
Date: Sun Feb 21 00:10:53 2010
New Revision: 45640

URL: http://svn.reactos.org/svn/reactos?rev=45640&view=rev
Log:
Partially revert patches 45626 and 45633.

Several services do not report their status to the service manager properly. Therefore we
must not use any code that relies on service status information as part of the setup and
boot processes as long as these issues have not been fixed. The service manager still
needs to provide fake information about the service status.

Modified:
    trunk/reactos/base/system/services/database.c
    trunk/reactos/dll/win32/syssetup/install.c

Modified: trunk/reactos/base/system/services/database.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/base/system/services/datab…
==============================================================================
--- trunk/reactos/base/system/services/database.c [iso-8859-1] (original)
+++ trunk/reactos/base/system/services/database.c [iso-8859-1] Sun Feb 21 00:10:53 2010
@@ -1054,7 +1054,7 @@
         {
             Group->ServicesRunning = TRUE;
         }
-        Service->Status.dwCurrentState = SERVICE_START_PENDING;
+        Service->Status.dwCurrentState = SERVICE_RUNNING;
     }
 #if 0
     else

Modified: trunk/reactos/dll/win32/syssetup/install.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/syssetup/install…
==============================================================================
--- trunk/reactos/dll/win32/syssetup/install.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/syssetup/install.c [iso-8859-1] Sun Feb 21 00:10:53 2010
@@ -473,34 +473,17 @@
 static BOOL
 EnableUserModePnpManager(VOID)
 {
-    SERVICE_STATUS_PROCESS ServiceStatus;
     SC_HANDLE hSCManager = NULL;
     SC_HANDLE hService = NULL;
-    DWORD dwStartTickCount;
-    DWORD dwOldCheckPoint;
-    DWORD BytesNeeded = 0;
-    DWORD dwWaitTime;
-    DWORD dwMaxWait;
-    HANDLE hEvent;
     BOOL ret = FALSE;
 
-    hEvent = OpenEventW(EVENT_ALL_ACCESS,
-                        FALSE,
-                        L"SC_AutoStartComplete");
-    if (hEvent == NULL)
-        goto cleanup;
-
-    WaitForSingleObject(hEvent, INFINITE);
-
-    hSCManager = OpenSCManager(NULL,
-                               NULL,
-                               SC_MANAGER_CONNECT);
+    hSCManager = OpenSCManager(NULL, NULL, 0);
     if (hSCManager == NULL)
         goto cleanup;
 
     hService = OpenServiceW(hSCManager,
                             L"PlugPlay",
-                            SERVICE_CHANGE_CONFIG | SERVICE_START |
SERVICE_QUERY_STATUS);
+                            SERVICE_CHANGE_CONFIG | SERVICE_START);
     if (hService == NULL)
         goto cleanup;
 
@@ -515,69 +498,9 @@
 
     ret = StartServiceW(hService, 0, NULL);
     if (!ret)
-    {
-        /* If the service is already running, just return TRUE */
-        ret = GetLastError() == ERROR_SERVICE_ALREADY_RUNNING;
         goto cleanup;
-    }
-
-    ret = QueryServiceStatusEx(hService,
-                               SC_STATUS_PROCESS_INFO,
-                               (LPBYTE)&ServiceStatus,
-                               sizeof(SERVICE_STATUS_PROCESS),
-                               &BytesNeeded);
-    if (!ret)
-        goto cleanup;
-
-    /* We don't want to wait for more than 30 seconds */
-    dwMaxWait = 30000;
-    dwStartTickCount = GetTickCount();
-
-    /* Loop until it's running */
-    while (ServiceStatus.dwCurrentState != SERVICE_RUNNING)
-    {
-        dwOldCheckPoint = ServiceStatus.dwCheckPoint;
-        dwWaitTime = ServiceStatus.dwWaitHint / 10;
-
-        /* Get the latest status info */
-        if (!QueryServiceStatusEx(hService,
-                                  SC_STATUS_PROCESS_INFO,
-                                  (LPBYTE)&ServiceStatus,
-                                  sizeof(SERVICE_STATUS_PROCESS),
-                                  &BytesNeeded))
-        {
-            /* Something went wrong... */
-            break;
-        }
-
-        /* Is the service making progress? */
-        if (ServiceStatus.dwCheckPoint > dwOldCheckPoint)
-        {
-            /* It is, get the latest tickcount to reset the max wait time */
-            dwStartTickCount = GetTickCount();
-            dwOldCheckPoint = ServiceStatus.dwCheckPoint;
-        }
-        else
-        {
-            /* It's not, make sure we haven't exceeded our wait time */
-            if (GetTickCount() >= dwStartTickCount + dwMaxWait)
-            {
-                /* We have, give up */
-                break;
-            }
-        }
-
-        /* Adjust the wait hint times */
-        if (dwWaitTime < 200)
-            dwWaitTime = 200;
-        else if (dwWaitTime > 10000)
-            dwWaitTime = 10000;
-
-        /* Wait before trying again */
-        Sleep(dwWaitTime);
-    }
-
-    ret = ServiceStatus.dwCurrentState == SERVICE_RUNNING;
+
+    ret = TRUE;
 
 cleanup:
     if (hSCManager != NULL)

[Eric Kohl]

Ged Murphy wrote:
>   Are these our services and if so do you have a note of
> which ones?
>  
>  
>  -----Original Message-----
>  From: ros-diffs-bounces(a)reactos.org [mailto:[e-mail]] On Behalf Of
> ekohl(a)svn.reactos.org
>  Sent: 20 February 2010 23:11
>  To: ros-diffs(a)reactos.org
>  Subject: [ros-diffs] [ekohl] 45640: Partially revert patches 45626 and 45633. Several
> services do not report their status to the service manager properly. Therefore we must not
> use any code that relies on service status information as part of the setup and boot pr
>  
>  Author: ekohl
>  Date: Sun Feb 21 00:10:53 2010
>  New Revision: 45640
>  
>  URL: http://svn.reactos.org/svn/reactos?rev=45640&view=rev
>  Log:
>  Partially revert patches 45626 and 45633.
>  
>  Several services do not report their status to the service manager properly. Therefore we
> must not use any code that relies on service status information as part of the setup and
> boot processes as long as these issues have not been fixed. The service manager still
> needs to provide fake information about the service status.
>  
>  Modified:
>      trunk/reactos/base/system/services/database.c
>      trunk/reactos/dll/win32/syssetup/install.c 
Yes, these are our services (in the meaning of: they are in our source 
tree).

The dhcp service is totally broken, besause it doesn't report anything 
back to the service manager, it doesn't even retrieve a service status 
handle. ;-( Rpcss, umpmpmgr and eventlog might need some work too but 
I'm not really sure about this because the service manager doesn't 
protect the service database from concurrent access yet.

I haven't checked the manually starting services yet.

These services seemed to run properly because the service mamager 
started the process of a given service, sent a start command to the 
services main thread and silently set the status to 'running'. With 
Dmitrys patches, the service manager sets the service status to 'start 
pending' and waits for the service to report the status changes.

Now, an application like 2nd stage setup may check umpnpmgr for example. 
It finds out that the service is 'start pending' and bails out, although 
the service is running properly. It just forgot to tell the service 
manager that it is running.

I hope this answers your question.

Now, I've got something to clean up...


Regards
Eric