alank2 5 Posted October 3 I tried Application->Terminate(); or Svcmgr::Application->Terminate();, but they do not exist. There is this, but I have no idea what the CtrlCode is - is there one for "stop" ? void __stdcall ServiceController(unsigned CtrlCode) { Service1->Controller(CtrlCode); } What is the proper way for a service application to request shutdown? I have a ServiceExecute method being called that contains this: while (!Terminated) { ServiceThread->ProcessRequests(true); Sleep(250); } I can't change Terminated to true, it says it is inaccessible. Share this post Link to post
Remy Lebeau 1392 Posted October 3 (edited) 36 minutes ago, alank2 said: I tried Application->Terminate() ; or S vcmgr::Application->Terminate();, but they do not exist. You can use ServiceThread->Terminate() instead. Quote Here is this, but I have no idea what the CtrlCode is - is there one for "stop" ? void __stdcall ServiceController(unsigned CtrlCode) { Service1->Controller(CtrlCode); } Yes, that will work, too. You can send it any control code that you can pass to ControlService(), such as SERVICE_CONTROL_STOP. Quote I have a ServiceExecute method being called that contains this: If that is all the code your OnExecute handler has then you don't need to have the handler assigned at all. When OnExecute is not assigned a handler, the service handles SCM requests automatically by default. When you assign an event handler, you become responsible for handling requests yourself. Edited October 3 by Remy Lebeau 1 Share this post Link to post
alank2 5 Posted October 3 1 hour ago, Remy Lebeau said: You can use ServiceThread->Terminate() instead. I called this from a timer method and it did not work. ServiceThread->Terminate(); 1 hour ago, Remy Lebeau said: Yes, that will work, too. You can send it any control code that you can pass to ControlService(), such as SERVICE_CONTROL_STOP. But this worked beautifully. ServiceController(SERVICE_CONTROL_STOP); 1 hour ago, Remy Lebeau said: If that is all the code your OnExecute handler has then you don't need to have the handler assigned at all. When OnExecute is not assigned a handler, the service handles SCM requests automatically by default. When you assign an event handler, you become responsible for handling requests yourself. Thanks for the info! I've got other init code in it and my loop also tests to see if the timer has finished executing before exiting the loop. I clear TProcess->Enabled when entering it and set it when it exits. log1.Log(L"Service started"); TProcess->Enabled=true; while (!Terminated || !TProcess->Enabled) //do not quit if process is running until it is finished { ServiceThread->ProcessRequests(true); Sleep(250); } TProcess->Enabled=false; log1.Log(L"Service STOPPED"); Share this post Link to post
Remy Lebeau 1392 Posted October 3 (edited) 4 hours ago, alank2 said: I called this from a timer method and it did not work. ServiceThread->Terminate(); That is because you are calling ProcessRequests() with the WaitForMessage parameter set to true. Not only is that blocking your while loop until a new SCM request arrives, but it also runs an internal loop that will not exit until a SERVICE_CONTROL_STOP request is received. ProcessRequests() will call Terminate() when processing a SERVICE_CONTROL_STOP request. So, your while loop will run AT MOST 1 iteration when using WaitForMessage=true. If you need to call ProcessRequests() yourself and be able to do other things in between calls (ie your Sleep(), etc) then you must call it with WaitForMessage=false instead, so it will exit immediately if there is no SCM request pending. Quote But this worked beautifully. ServiceController(SERVICE_CONTROL_STOP); Correct, because you are satisfying the condition that ProcessRequests() is waiting for. Quote Thanks for the info! I've got other init code in it and my loop also tests to see if the timer has finished executing before exiting the loop. I clear TProcess->Enabled when entering it and set it when it exits. See above for why that approach will not work as you are expecting. Edited October 3 by Remy Lebeau 1 Share this post Link to post
alank2 5 Posted October 4 (edited) still editing Edited October 4 by alank2 Share this post Link to post
alank2 5 Posted October 4 16 hours ago, Remy Lebeau said: Not only is that blocking your while loop until a new SCM request arrives, but it also runs an internal loop that will not exit until a SERVICE_CONTROL_STOP request is received. ProcessRequests() will call Terminate() when processing a SERVICE_CONTROL_STOP request. So, your while loop will run AT MOST 1 iteration when using WaitForMessage=true. The sleep isn't really important as it was just added to keep it from consuming the CPU if ServiceThread->ProcessRequests(true); returns immediately. I am okay with ServiceThread->ProcessRequests(true); not returning until the service is stopped, the loop only needs to come into play during that situation where the "while (!Terminated || !TProcess->Enabled)" keeps it running until TProcess->Enabled becomes true (meaning that that method has successfully completed). The goal there is to not have the service stop without waiting for the TProcess method to complete. Maybe the 250ms should be dropped to 1ms in the Sleep as that could be keeping the ServiceThread->ProcessRequests(true); from executing more quickly to help it get stopped. So, my ServiceExecute does (1) init code, (2) ServiceThread->ProcessRequests until terminated and my tprocess timer method is complete, and finally (3) termination code. Is there a better way to do it? Should I move 1 into OnStart and 2 into OnStop? Can I eliminate ServiceExecute but still have it wait on the tprocess timer method to complete before executing OnStop and shutting down? Share this post Link to post
Remy Lebeau 1392 Posted October 4 (edited) 7 hours ago, alank2 said: I am okay with ServiceThread->ProcessRequests(true); not returning until the service is stopped, the loop only needs to come into play during that situation where the "while (!Terminated || !TProcess->Enabled)" keeps it running until TProcess->Enabled becomes true (meaning that that method has successfully completed). In order to use a loop correctly in the OnExecute event, you must call ProcessRequests() with WaitForMessage=false. Using WaitForMessage=true, ProcessRequests() will not return until the service is stopped, and the stop will set Terminated=true, thus making such a loop useless. 7 hours ago, alank2 said: The goal there is to not have the service stop without waiting for the TProcess method to complete. If you want to prevent the service from being stopped at all while the TProcess is busy, then you can do one of the following: when starting the TProcess, set the service's AllowStop property to false and call the ReportStatus() to update the SCM, and then once the TProcess has finished you can set AllowStop back to true and call ReportStatus() again. leave AllowStop set to true, and have the OnStop event return Stopped=false (and set the service's Win32ErrCode or ErrCode property accordingly) if the TProcess is currently busy. On the other hand, if you want the service to stop normally but wait for the TProcess to finish, then you can handle that in the OnStop event. The service's Status is already csStopPending when the OnStop event is entered. Simply run a loop inside of the OnStop event to call ReportStatus() at regular intervals (not exceeding the WaitHint property) while the TProcess is busy so the SCM knows the service is not frozen. Once the TProcess has finished, then OnStop can return Stopped=true, which will update the SCM with a status of csStopped. 7 hours ago, alank2 said: Maybe the 250ms should be dropped to 1ms in the Sleep as that could be keeping the ServiceThread->ProcessRequests(true); from executing more quickly to help it get stopped. As I explained earlier, the OnExecute code you have shown is basically useless and should be eliminated completely. 7 hours ago, alank2 said: So, my ServiceExecute does (1) init code, (2) ServiceThread->ProcessRequests until terminated and my tprocess timer method is complete, and finally (3) termination code. I would not use that approach. The init code should be in the OnStart event. The termination code should be in OnStop event. The OnExecute event should be eliminated whenever possible. Quote Is there a better way to do it? Should I move 1 into OnStart and 2 into OnStop? Yes, and yes. Quote Can I eliminate ServiceExecute but still have it wait on the tprocess timer method to complete before executing OnStop and shutting down? Yes. For example: void __fastcall TService1::ServiceStart(TObject *Sender, bool &Started) { TProcess->Enabled = true; log1.Log(L"Service started"); Started = true; } void __fastcall TService1::ServiceStop(TObject *Sender, bool &Stopped) { while (TProcess->Enabled) { Sleep(WaitHint-100); ReportStatus(); } log1.Log(L"Service STOPPED"); Stopped = true; } void __fastcall TService1::TProcessTimer(TObject *Sender) { ... if (some condition) { TProcess->Enabled = false; if (Status != csStopPending) ServiceController(SERVICE_CONTROL_STOP); } ... } One thing to keep in mind - you keep saying the TProcess is a "timer method". If you mean a TTimer, then such a loop in the OnStop event would block the timer from firing any further OnTimer events, unless you manually pump the service thread's message queue to dispatch WM_TIMER messages. You would have had to do that anyway in the OnExecute event, if you start the timer in the context of the service thread. Personally, I never do any work in the service thread directly. I always have the OnStart event create a worker thread, and then have the OnStop event terminate the worker thread and wait on it. If I need to do things at regular intervals, I have the worker thread use a Waitable Timer Object or Waitable Event Object in a loop. Edited October 4 by Remy Lebeau 2 Share this post Link to post
alank2 5 Posted October 5 Thanks Remy; I'll work through this! I appreciate it. Share this post Link to post