Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am doing multithreading using TPL Tasks first time. This is what i have done so far and I want to do it the right way. I will really appreciate if you could honor me with your expert advice.

I am developing a C# windows service that will always watch different folders/files and DB query results on different time intervals. There can be dozens of watchers each watching a specific file or folder or DB query results and sends emails at specific email address if some predefined threshold is met. And begins watching again......

So the requirements that I m trying to address are:

  1. Each Watcher must do its task in a separate Thread
  2. If a watcher/tasks throws an exception ... it must automatically be restarted after ,say, 3 hours

The strategy I am using is as follows:

  1. I create a List of IWatcher objects which holds many instances of DB, File and Folder watchers

    IWatcher is just an interface with some common properties/methods across all DB/File/Folder watchers

  2. I call a method "BeginWatch" which creates a separate task for each watcher in watcher's list and stored that task in a Dictionar... the key is set to the ID of watcher.

  3. I surrounded thread method's body with try/catch to catch any exception.

  4. if an exception occurs, i create a Timer object in catch() body, save the watcher's ID (whose work is stopped) in this timer and schedule it with its interval set to a few hours. I remove the task object from tasks' dictionary.

  5. Upon occurring of the 'Elapsed' event of this timer, I recreate another task and add it to tasks' dictionary.

So, the code goes as follows:

Dictionary object for holding tasks against a watcher's ID!

    private Dictionary<string, TaskDetails> _watcherThreads

Here

string: is DB/File or Folder watcher's unique ID

TaskDetails: A class that holds a `Task` and other information for that task that I need during  program execution.

TaskDetails is as follows:

class TaskDetails
{        
    //The Task object
    public Task WatcherTask { get; set; }

    //CancellationTokenSource reference for each task .. 
    //if we need to cancel tasks individually
    public CancellationTokenSource WatcherCancellationToken { get; set; }

    //It a Timer that will enable this specific task after a few hours 
    //if an exception occurs 
    public MyTimer DisablingTimer { get; set; }

    public TaskDetails(
        Task task, 
        CancellationTokenSource cancellationToken)
    {
        this.WatcherTask = task;
        this.WatcherCancellationToken = cancellationToken;
    }
}

Next I create a list of watchers and then call the following method to create a task for each watcher

public void BeginWatch()
    {
        //this._watchers is the List<IWatcher> where all the watcher objects are stored
        if (this._watchers == null || this._watchers.Count == 0)
            throw new ArgumentNullException("Watchers' not found");

        //call CreateWatcherThread for each watcher to create a list of threads
        this._watchers.ForEach(CreateWatcherThread);

    }

I create a list of tasks with other info as follows:

//I call this method and pass in any watcher which i want to run in a new thread
//This method is called savaral times for creating a TASK for each IWatcher object
private void CreateWatcherThread(IWatcher watcher)
{
    IWatcher tmpWatcher = watcher.Copy();
    CancellationTokenSource cancellationToken = new CancellationTokenSource();

    //Create a task and run it
    Task _watcherTask = Task.Factory.StartNew(
            () => _createWatcherThread(tmpWatcher, cancellationToken),
            cancellationToken.Token, 
            TaskCreationOptions.LongRunning, 
            TaskScheduler.Default);

    //Add Thread, CancellationToken and IsDisabled = false to dictionary
    //Save Key as WID that will be unique. this will help us retrieving 
    //the right TASK object later from the list 
    this._watcherThreads.Add(
            tmpWatcher.WID,
            new TaskDetails(_watcherTask, cancellationToken)
        );
}

thread method -- will perform the operation. and will set a timer to enable this watcher after some time IF an exception occurs

private void _createWatcherThread(IWatcher wat
                                   , CancellationTokenSource cancellationToken)
{   IWatcher watcher = wat.Copy();
    bool hasWatchBegin = false;
    try
    { 
        //run forever
        for (;;)
        {
            //dispose the watcher and stop this thread if CANCEL token has been issued
            if (cancellationToken.IsCancellationRequested)
            {  ((IDisposable)watcher).Dispose();
                break;
            }
            else if (!hasWatchBegin)
            {   watcher.BeginWatch();
                hasWatchBegin = true;
            }
        }
    }
    catch (Exception ex)
    { 
        //set timer to reactivate this watcher after some hours
        ///This is an extended System.Timers.Timer class. I have only added WatcherID property in it to associate it with a specific Watcher object
        MyTimer enableTaskTimer = new MyTimer();
        enableTaskTimer.Interval 
              = AppSettings.DisabledWatcherDuration; // say 3 hours

        // Store Watcher's ID so we can create another Task later and it begins running this watcher
        enableTaskTimer.WatcherID = watcher.WID;
        enableTaskTimer.Elapsed += DisablingTimer_Elapsed;
        enableTaskTimer.Start();

        //remove the thread from existing list as this task will be recreated again and stored in the list
        this._watcherThreads.Remove(watcher.WID);

        //Log exception
        SingletonLogger.Instance.WriteToLogs(ex.Message, LogSeverity.Error);
    }
}

void DisablingTimer_Elapsed (object sender, System.Timers.ElapsedEventArgs e)
{   
    MyTimer timer = sender as MyTimer;

    // get the watcher object by its WID.. MyTime .WatcherID has the Id of the watcher that crashed ... now its time to recreate a task that will again begin a watch
    IWatcher wat = this._watchers.Where(w => w.WID == timer.WatcherID).SingleOrDefault();

    //dispose tie timer ... no more needed
    timer.Stop();
    timer.Dispose();

    //recreate a new thread for watcher
    if(wat != null)
        this.CreateWatcherThread(wat);
}

Am I doing this the right way?

share|improve this question
No suggestions ??? hmmm... then I think I m a perfect programmer – Aamir May 11 at 10:26

Know someone who can answer? Share a link to this question via email, Google+, Twitter, or Facebook.

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.