trjhtr

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
 Thread.Sleep(2000);
 return true;
 
 else
 
 //sleep for 10 seconds before trying again
 Thread.Sleep(10000);
 //if the elapsed time is more than 10 minutes return false
 if (start < DateTime.Now.AddSeconds(-600))
 
 wcfmonitoring.Close();
 return false;
 
 
 
 
 catch
 
 wcfmonitoring.Abort();
 
 
 //return false in case of error
 return false;

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 = 
 CancellationTokenSource.CreateLinkedTokenSource(localCancellation.Token, 
 globalCancellation.HasValue ? 
 globalCancellation.Value : 
 CancellationToken.None);

 Task cancelationTask = null;
 Task actionTask = null;

 try
 
 cancelationTask = Task.Delay(timeout, localAndGlobalCancellation);
 actionTask = Task.Factory.StartNew(() =>
 
 result = action.Invoke();
 , localAndGlobalCancellation);
 
 catch (Exception ex)
 
 Trace.WriteLine(ex.Message);
 localCancellation.Cancel();
 return null;
 

 await Task.WhenAny(actionTask, cancelationTask).ContinueWith(t => localCancellation.Cancel());
 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
 

 Thread.Sleep(2000); //for the importer
 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?