views:

248

answers:

7

I'm working in a service whose main loop looks like this:

while (fServer.ServerState = ssStarted) and (Self.Terminated = false) do
begin
  Self.ServiceThread.ProcessRequests(false);
  ProcessFiles;
  Sleep(3000);      
end;

ProcessRequests is a lot like Application.ProcessMessages. I can't pass true to it because if I do then it blocks until a message is received from Windows, and ProcessFiles won't run, and it has to run continually. The Sleep is there to keep the CPU usage down.

This works just fine until I try to shut down the service from Windows's service management list. When I hit Stop, it sends a message and expects to get a response almost immediately, and if it's in the middle of that Sleep command, Windows will give me an error that the service didn't respond to the Stop command.

So what I need is to say "Sleep for 3000 or until you receive a message, whichever comes first." I'm sure there's an API for that, but I'm not sure what it is. Does anyone know?

+2  A: 

Use a timer to run ProcessFiles instead of hacking it into main application loop. Then ProcessFiles will run in the interval you want and the messages will be processed correctly, not taking 100 % CPU.

dark_charlie
That was the first thing I tried, and it didn't work. I took it out and replaced it with the sleep loop, which I agree is an ugly hack. But I just tried it again with a timer, (a lot's changed since I did that) and now it works. Go figure.
Mason Wheeler
This is not appropriate for a service
mj2008
@mj2008: This wasn't supposed to be a service at all, originally. But you know how scope creep is... :(
Mason Wheeler
There are timers that work well with services, see for example http://stackoverflow.com/questions/246697/windows-service-and-timer (it's not a Delphi code though). I think the Delphi timer could work fine anyway.
dark_charlie
Maybe in this case, but as soon as you start using threads, the TTimer has a bag of surprises for you.
ErvinS
Then you depend on mainthread regular availability. Not a good thing in busy apps.
Marco van de Voort
It so happens that the periodic TTimer firing in a VCL app is a lurking source of really fun stuff. Especially come the day you figure you should add a call to Application.ProcessMessages somewhere in your application code. Remember this day, when the next weird bug shows up.
Warren P
A: 

You don't need to sleep for 3 full seconds to keep the CPU usage low. Even something like Sleep(500) should keep your usage pretty low (if there are no messages waiting to process it should blow through the loop pretty quick and hit the sleep again. If your loop takes a few ms to run it still means your thread is spending the vast majority of time in sleep.

That being said, your code may benefit from some refactoring. You say you don't want ProcessRequests to block waiting for a message? The only other thing in that loop is ProcessFiles. If that is dependent on the message being processed then why can't it block? And if it's not dependent on the message being processed then can it be split onto another thread? (the previous suggestion of firing ProcessFiles via a timer is an excellent suggestion on how to do this).

Zippit
+10  A: 

This kind of stuff is hard to get right, so I usually start at the API documentation at MSDN.

The WaitForSingleObject documention specifically directs to MsgWaitForMultipleObjects for these kinds of situations:

Use caution when calling the wait functions and code that directly or indirectly creates windows. If a thread creates any windows, it must process messages. Message broadcasts are sent to all windows in the system. A thread that uses a wait function with no time-out interval may cause the system to become deadlocked. Two examples of code that indirectly creates windows are DDE and the CoInitialize function. Therefore, if you have a thread that creates windows, use MsgWaitForMultipleObjects or MsgWaitForMultipleObjectsEx, rather than WaitForSingleObject.

In MsgWaitForMultipleObjects, you have a dwWakeMask parameter specifying on which queued messages to return, and a table describing the masks you can use.

Edit because of comment by Warren P:

If your main loop can be continued because of a ReadFileEx, WriteFileEx or QueueUserAPC, then you can use SleepEx.

--jeroen

Jeroen Pluimers
This is the smart way to go. The other ways are not really "waiting" in a background thread, they are periodically doing something in a foreground thread. These two things are not alternatives for doing the same thing, they are radically different things. Use a thread or don't use a thread, but if you're using a thread, and waiting on a message, you do need it to be interruptable. SleepEx is also interruptable.
Warren P
+1  A: 

I used a TTimer in a multithreaded application with strange results, so now i use Events.

while (fServer.ServerState = ssStarted) and (Self.Terminated = false) do
begin
  Self.ServiceThread.ProcessRequests(false);
  ProcessFiles;

  if ExitEvent.WaitFor(3000) <> wrTimeout then
    Exit;   
 end;

You create the event with

ExitEvent := TEvent.Create(nil, False, False, '');

Now the last thing is to fire the event in case of service stop. I think the Stop event of the service is the right place to put this.

ExitEvent.SetEvent;

I use this code for an cleanup thread in my DB connections pooling system, but it should work well in your case too.

ErvinS
That suffers from the same problem as the original - it is waiting the full 3 seconds since ExitEvent is not able to be signaled until ProcessRequests() is called to process the SCM's stop request.
Remy Lebeau - TeamB
A: 

Use an TEvent that you signal when the thread should wake up. Then block on the tevent (using waitformultiple as Jeroen says if you have multiple events to wait on)

Marco van de Voort
+5  A: 

MsgWaitForMultipleObjects() is the way to go, ie:

while (fServer.ServerState = ssStarted) and (not Self.Terminated) do 
begin 
  ProcessFiles; 
  if MsgWaitForMultipleObjects(0, nil, FALSE, 3000, QS_ALLINPUT) = WAIT_OBJECT_0 then
    Self.ServiceThread.ProcessRequests(false); 
end;

If you want to call ProcessFiles() at 3 second intervals regardless of any messages arriving, then you can use a waitable timer for that, ie:

var
  iDue: TLargeInteger;
  hTimer: array[0..0] of THandle;
begin
  iDue := -30000000; // 3 second relative interval, specified in nanoseconds
  hTimer[0] := CreateWaitableTimer(nil, False, nil);
  SetWaitableTimer(hTimer[0], iDue, 0, nil, nil, False);
  while (fServer.ServerState = ssStarted) and (not Self.Terminated) do 
  begin 
    // using a timeout interval so the loop conditions can still be checked periodically
    case MsgWaitForMultipleObjects(1, hTimer, False, 1000, QS_ALLINPUT) of
      WAIT_OBJECT_0:
      begin
        ProcessFiles;
        SetWaitableTimer(hTimer[0], iDue, 0, nil, nil, False);
      end;
      WAIT_OBJECT_0+1: Self.ServiceThread.ProcessRequests(false);
    end;
  end;
  CancelWaitableTimer(hTimer[0]);
  CloseHandle(hTimer[0]);
end;
Remy Lebeau - TeamB
A: 

Is it not possible to move ProcessFiles to a seperate thread? In your MainThread you just wait for messages and when the service is being terminated you terminate the ProcessFiles thread.

The_Fox
Already got that covered. ProcessFiles basically just looks to see if there are any files to be processed and hands them off to another thread.
Mason Wheeler
@Mason Wheeler: That doesn't hold you off to move ProcessFiles to another thread, or maybe let the file processing thread call ProcessFiles itself after it is done.
The_Fox