# Long running operation with timeout

I have a long running operation that I need to wait for or timeout. I have working code, but I would like your opinion on improving it.

public bool RegisterEquipmentListUpdate(int ID)
{
if (Authentication.CheckSession(Session))
{
var wcfmonitoring = new WCFMonitoring.MonitoringDatabaseClient();
try
{
bool timeout = false;
DateTime start = DateTime.Now;
//loop as long as the time isn't reached (600 seconds)
while (!timeout)
{
if (wcfmonitoring.CheckForEquipmentUpdate(Authentication.GetSessionID(Session), Authentication.GetPTO(Session), ID))
{
wcfmonitoring.Close();
//sleep before returning true, this is set so the importer can finish multiple files without the client refreshing on the first one
return true;
}
else
{
//sleep for 10 seconds before trying again
//if the elapsed time is more than 10 minutes return false
{
wcfmonitoring.Close();
return false;
}
}
}
}
catch
{
wcfmonitoring.Abort();
}
}
//return false in case of error
return false;
}

• have you considered async/await – Nkosi Mar 2 '18 at 12:45
• Yes which would be a lot cleaner than blocking thread – Nkosi Mar 2 '18 at 13:19
• You could also consider changing the approach and using a timer as apposed to a busy wait....just thinking aloud here – Nkosi Mar 2 '18 at 13:44

This is a rough draft, since I do not know the overall picture of your project. Some design might be improved to not even use this code (events, semaphores, mutexes, and other more predictable synchronization mechanisms). I rarely use this pattern for code that I control, more for code I cannot that I know is blocking. This approach is risky, as it can lead to undesired state and data loss. You have been warned! (http://thedailywtf.com/articles/My-Tales) for reference!)

If you stay on this path, I recommend A separate class/method be responsible for this sort of behaviour, to favour reuse, this also includes the possibility of having a CancellationToken passed in, just in case you change your mind about the request:

public async Task<T> WaitForActionCompletionOrTimeout<T>(Func<T> action, int timeout, CancellationToken? globalCancellation = null)
{
var result = default(T); //return default for error

var localCancellation = new CancellationTokenSource();
var localAndGlobalCancellation =
globalCancellation.HasValue ?
globalCancellation.Value :
CancellationToken.None);

try
{
{
result = action.Invoke();
}, localAndGlobalCancellation);
}
catch (Exception ex)
{
Trace.WriteLine(ex.Message);
localCancellation.Cancel();
return null;
}

return result;
}


Change the work method to return true or false, something like:

public bool CheckForUpdate()
{
using (var wcfmonitoring = new WCFMonitoring.MonitoringDatabaseClient())
{
var sw = new SpinWait();
while (!wcfmonitoring.CheckForEquipmentUpdate(Authentication.GetSessionID(Session), Authentication.GetPTO(Session), ID)) //maybe  something shorter would work :) some variables are nice for debugging
{
sw.SpinOnce();  //magic Thread.Sleep(5) replacement with lots of goodies :) nice class to look at
}

return true;
}
}


And then:

var success = WaitForActionCompletionOrTimeout(() => CheckForUpdate, 600000 ); //I would so like 6.minutes.in.miliseconds.... ah dreams


This sort of approach (cancelling the worker) could also be done with threads in a much shorter and brute-forcier way for a slow performance increase in case of very low timeouts.

I would also advise using using and if WcfMonitoring is not IDisposable, to make it IDisposable.

One more question: What is the intent/difference between calling Abort and Close in your code?