Simple selfwritten JobScheduler causes deadlock?-CodePudding

Im writing my own JobScheduler for learning purposes. The idea is pretty simple, it starts n threads, pulls Jobs/Task from a concurrent queue, processes them and once finished it will notify a Event so the mainthread can wait for it to finish ( if he wants ).

The thread loop looks like this...


    internal long ItemCount;  // The amount of jobs to process
    internal ManualResetEventSlim Event { get; set; }  // Event to notify worker threads for new items 
    internal ConcurrentQueue<JobMeta> Jobs { get; set; }  // Jobs

    private void Loop(CancellationToken token) {
        
        Loop:

        // Break if cancellation is requested
        if (token.IsCancellationRequested) return;
        
        // Make threads wait, the event tells them when new jobs arrived
        Event.Wait(token);
        if (Jobs.TryDequeue(out var jobMeta)) {  // Concurrent, dequeue one at a time

            // Make other threads wait once no more items are in the queue
            if(Interlocked.Decrement(ref ItemCount) == 0) Event.Reset();

            jobMeta.Job.Execute();      // Execute job
            jobMeta.JobHandle.Set(); // ManualResetEvent.Set to notify the main e.g.
        }
        goto Loop;
    }

    // Notify threads about new arrived jobs 
    public void NotifyThreads() {
        Interlocked.Exchange(ref ItemCount, Jobs.Count);  // Set ItemCount
        Event.Set();  // Notify
    }

    // Enqueues new job
    public JobHandle Schedule(IJob job) {

        var handle = new ManualResetEvent(false);
        var jobMeta = new JobMeta{ JobHandle = handle, Job = job};
        Jobs.Enqueue(jobMeta);
        return handle;
    }

However sometimes this causes a deadlock if i do something like this :

var jobHandle = threadScheduler.Schedule(myJob);  // JobHandle is a ManualResetEvent
threadScheduler.NotifyThreads();

for(var index = 0; index < 10000; index  ){
   
   var otherJobHandle = threadScheduler.Schedule(otherJob);
   threadScheduler.NotifyThreads();
   otherJobHandle.Wait();
}

jobHandle.Wait();  // Deadlock sometimes...

Why could this cause a deadlock ? Wheres the logic issue ? And how would a normal JobScheduler look like ( since i cant find any good informations about this topic in general ) ?

Glad for any help !

CodePudding user response：

I don't think your last snippet could cause a deadlock, because jobHandle is first enqueued, its handle should already be set.

I can find 2 problems in your implemention. First is this line:

Interlocked.Exchange(ref ItemCount, Jobs.Count);

Think about if the code is executed in the following order:

                         //queue.count = 1
1. Jobs.TryDequeue       //queue.count = 0
2. Interlocked.Exchange  //ItemCount = 0
3. Interlocked.Decrement //ItemCount = -1

ItemCount is decreased twice. So what you should do is increase ItemCount while a job is enqueued.

Interlocked.Increment(ref ItemCount);
Jobs.Enqueue(jobMeta);

The second problem is in the loop, think about this code execution order:

1. Jobs.TryDequeue
2. Interlocked.Decrement
  3. Interlocked.Increment
  3. Jobs.Enqueue
  3. Event.Set
4. Event.Reset

In the end the state of the wait handle is unset, it can cause a deadlock because your attempt is the handle should be set after enqueueing a job. In such case you need run the code in a reversed order:

1. Jobs.TryDequeue
2. Event.Reset // anyway reset the handle
3. if(Interlocked.Decrement > 0)
      Event.Set

With this approach, whenever a job is enqueued before Interlocked.Decrement or not, the state of the wait handle is correct.

Above approach maybe is not efficient enough because we need toggle the wait handle in each loop, I suggest you to add a while-loop for the queue:

int count = 0;
while(Jobs.TryDequeue(out var jobMeta)) {
      count;
    ...
}

if(count > 0) {
    Event.Reset();
    if(Interlocked.Add(ref ItemCount, -count) > 0)
        Event.Set();
}