Job Scheduler Implementation

Question

I have created a Job Scheduler, and i want some review, new ideas for it.

Scheduler.cs

public abstract class Scheduler
{
    #region Fields && Properties

    private readonly List<Job> _jobs = new List<Job>();
    private readonly Random _rand = new Random();
    private Job _currentlyExecutingJob;
    private Thread _workingThread;
    public bool? Parallel { get; private set; }
    public DateTimeOffset NextExecutionTime { get; private set; }
    public string ID { get; set; }
    public abstract Task JobTrigger(Job job);
    public abstract void UnobservedException(Exception exception, Job job);

    #endregion

    #region Ctor

    protected Scheduler(bool parallel) { Parallel = parallel; }

    #endregion

    #region Methods

    public void Start()
    {
        if (!Equals(_workingThread, null)) return;
        _workingThread = new Thread(ReviewJobs);
        _workingThread.Start();
    }

    public void Stop()
    {
        if (Equals(_workingThread, null)) return;
        _workingThread.Abort();
        _workingThread = null;
    }

    private void ReviewJobs()
    {
        while (!Equals(_workingThread, null)) {
            IEnumerable<Job> jobsToExecute = from job in _jobs
                                             where job.NextExecutionTime <= DateTimeOffset.Now
                                             orderby job.Priority
                                             select job;

            if (!jobsToExecute.Any()) {
                Thread.Sleep(100);
                continue;
            }

            try {
                foreach (Job job in jobsToExecute) {
                    Job o = _currentlyExecutingJob = job;

                    if (Parallel != null && (bool)Parallel) {
                        JobTrigger(o);
                    } else {
                        JobTrigger(o).Wait();
                    }

                    if (!o.Repeat)
                        _jobs.Remove(o);
                    else if (o.Interval != null)
                        o.NextExecutionTime = DateTimeOffset.Now.Add((TimeSpan)(o.RandomizeExecution
                                                                                        ? TimeSpan.FromSeconds(_rand.Next((int)((TimeSpan)o.Interval).TotalSeconds, ((int)((TimeSpan)o.Interval).TotalSeconds + 30)))
                                                                                        : o.Interval));
                }
            } catch (Exception exception) {
                UnobservedException(exception, _currentlyExecutingJob);
            } finally {
                NextExecutionTime = (from job in _jobs
                                     where job.NextExecutionTime <= DateTimeOffset.Now
                                     orderby job.Priority
                                     select job.NextExecutionTime).FirstOrDefault();
            }
        }
    }

    #endregion

    #region Helper Methods

    public Job GetJob(string id) { return _jobs.Find(job => job.ID == id); }

    public Job GetJob(Job job) { return _jobs.Find(x => x == job); }

    public Job GetJob<T>(Job<T> job) { return _jobs.Find(x => x == job); }

    public IEnumerable<Job> GetAllJobs() { return _jobs; }

    public void RemoveJob(string id)
    {
        Job existingJob = GetJob(id);

        if (null != existingJob)
        {
            _jobs.Remove(existingJob);
        }
    }

    public void RemoveJob(Job job)
    {
        Job existingJob = GetJob(job);

        if (null != existingJob)
        {
            _jobs.Remove(existingJob);
        }
    }

    public void RemoveAllJobs() { _jobs.RemoveRange(0, _jobs.Count); }

    #endregion
}

Job.cs

public class Job
{
    public string ID { get; set; }
    public TimeSpan Interval { get; private set; }
    public DateTimeOffset NextExecutionTime { get; set; }
    public int Priority { get; set; }
    public bool Repeat { get; private set; }
    public bool RandomizeExecution { get; set; }
    public object Data { get; set; }

    #region Fluent

    public Job RunOnceAt(DateTimeOffset executionTime)
    {
        NextExecutionTime = executionTime;
        Repeat = false;
        return this;
    }

    public Job RepeatFrom(DateTimeOffset executionTime, TimeSpan interval)
    {
        NextExecutionTime = executionTime;
        Interval = interval;
        Repeat = true;
        return this;
    }

    #endregion
}

Usage:

public class SchedulerUsage : Scheduler
{
    public SchedulerUsage(bool parallel) : base(parallel) {
    }

    public override async Task JobTrigger(Job job) 
    {
        switch (job.ID) {
                //Handle Jobs
        }
    }

    public override void UnobservedException(Exception exception, Job job) 
    { 
        //Handle Exceptions
    }

    /// <summary>
    /// Example of adding job
    /// </summary>
    public void ExampleUsage()
    {
        Job job = new Job
                  {
                          ID = "ID",
                          Data = "ATTACH SOME DATA"
                  }.RunOnceAt(DateTimeOffset.Now.AddSeconds(7));

        //Add the job... [HERE IS WHAT I WANT TO ACHIVE]
        /*await*/base.AddJob(job);

        //Start the scheduler...
        base.Start();
    }
}

Also Please i want to know how could i use Async/Await to wait for a job to finish executing ?. [VERY IMPORTANT]

Answer 1

1. Using Equals(object, null) to test whether an object is null is rather unusual. I can't see any reason in your case why it would be preferred over object == null.
2. Why is Parallel a nullable bool? I can't see any code which relies of the tri-state nature. If you insist on keeping it then a more concise way of checking it for true is if (Parallel == true) which will only evaluate to true if it is not null and true.
3. Parallel is not a good name for a boolean flag. RunJobsInParallel is a bit longer but makes it much more clear what the flag is doing.
4. Calling Abort() to terminate a thread is nasty. Use a flag which the main scheduler methods checks at convenient points in order to exit early. Use Join with a timeout to wait for the worker thread.
5. _jobs is not thread safe. You iterate over it in ReviewJobs on the worker thread and you have public remove methods which could be called from a different thread to remove jobs from the list. This is waiting for an exception to happen ("collection was modified during enumeration").
6. Actually upon reading your code more carefully: jobsToExecute is an IEnumerable and because it's built by a LINQ statement this means you "only" have a set of nested enumerators which iterate over the original _jobs collection when you do your foreach. This means your code should fail when you call _jobs.Remove() inside that loop. jobsToExecute needs to be built with ToList() or ToArray() to create a independent copy.

7. If you want to wait for specific jobs to finish then you can put a ManualResetEvent on the Job class. This should be set by whoever executes the job once it is finished. Expose a WaitAsync() method on the Job class to wait on the event. Something along these lines:

   public class Job
   {
       ...
       private ManualResetEventSlim _JobDone = new ManualResetEventSlim(false);
   
       public void ProcessingStarted()
       {
           _JobDone.Reset();
       }
   
       public void ProcessingDone()
       {
           _JobDone.Set();
       }
   
       public async Task WaitAsync()
       {
           await Task.Run(() => _JobDone.WaitOne());
       }
   }
   

   JobTrigger should do something along these lines:

   job.ProcessingStarted();
   ... // process job
   job.ProcessingDone();
   

   Then something can wait on the job. As mentioned by svick the above code might not scale well if you have many jobs to wait on. Try StackOverflow if you need help solving this specific problem.