0

I have a server which listens to clients in a while(true) loop. I keep every client's hostname in a list and save the time the client contact the server. I would like to check every 10 minutes if some of the clients didn't contact the server in the last hour and to print its name. I thought about doing something like this:

Task.Run(CheckTheClients()) //Check the passed-time of each client in the list
while(true)
{
//listen to clients, add them to list, etc.
}

But I'm not sure how to do the check every 10 minutes and not every millisecond, neither if my idea is good or not. So What is the best way to do this? Moreover, both the function and the while(true) touches the list of the clients. Is that going to make some problems?

CleverPatrick
  • 9,261
  • 5
  • 63
  • 86
Amir B
  • 41
  • 6
  • 1
    You mean, like a timer? https://stackoverflow.com/questions/12535722/what-is-the-best-way-to-implement-a-timer –  Nov 26 '19 at 19:45
  • @Amy Thanks. Does the shared clients' list going to cause some problems? – Amir B Nov 26 '19 at 19:55
  • That depends entirely on what you do with it. Use a mutex or a concurrent collection if necessary. –  Nov 26 '19 at 20:00

3 Answers3

4

This would be best done by using the Timer function, basically you create it, pass it a function to call at each amount of time passed, and set the time to wait in Milliseconds. So for your example of 10 minutes, something like this:

// insert this into a long running function, and scope the timer variable correctly 
System.Timers.Timer myTimer = new System.Timers.Timer();
myTimer.Elapsed += new ElapsedEventHandler(OnTimedEvent);
myTimer.Interval = 600000;
myTimer.Enabled = true;


 // Define what you want to happen when the Elapsed event occurs (happens on the interval you set).
 private static void OnTimedEvent(object source, ElapsedEventArgs e)
 {
     //do some work here
 }
OperatorOverload
  • 724
  • 4
  • 17
  • Thanks. What about the shared resource, the clients' list. Won't cause any troubles? – Amir B Nov 26 '19 at 19:54
  • 1
    From what I'm imagining, the client list isn't really a shared resource because the server is the only thing maintaining the list. If you have multiple threads running this then you'd need to plan accordingly, generally with a Concurrent collection (https://learn.microsoft.com/en-us/dotnet/api/system.collections.concurrent?view=netframework-4.8). If you have another thread that's working with the collection then you have this timer using it in the background, you should definitely use a concurrent to ensure no deadlocks. – OperatorOverload Nov 26 '19 at 20:03
  • Going with a TPL-based pattern for this instead of a Timer might help with concurrency issues – Josh E Nov 26 '19 at 20:10
1

You can put your thread to sleep, like this:

while (true)
{
   try
   {
        // do something
   }
   catch (Exception ex)
   {
        // save log 
   }

   Thread.Sleep(TimeSpan.FromMilliseconds(TimeSpan.FromMinutes(10).TotalMilliseconds));
}
1

Since you're using Task.Run in the sample code you provided, why not use Task.Delay while you're at it?

Action CheckTheClients = () => Console.WriteLine("Checking clients...");
while (true)
{
    var task = Task.Delay(1000).ContinueWith(x => CheckTheClients());
    await task;
}

No need to sign up for timer and its events, if you're going for simplicity.

Josh E
  • 7,390
  • 2
  • 32
  • 44