0

I want to create a working thread that starts on signal(task was added to shared task list), and will rest when done.

Requirements:

  1. Other system threads can add task any time
  2. the working thread should rest if it has nothing to-do
  3. if more tasks are added while the working thread is active, it should complete them too.
  4. working thread can rest for hours -> work arrive like raindrops(sometime we have a storm)

After thread adds new task to the workingList(shared list) it signals(AutoRestEvent.Set()) the working-thread to start working.

I have race condition between the Set() and the WaitOne() functions.

    public static void AddWork(object obj)
    {
        Monitor.Enter(_syncO);
        _workingList.Add(obj);
        _signal.Set();
        Monitor.Exit(_syncO);
    }

    static object _syncO = new object();
    static AutoResetEvent _signal = new AutoResetEvent(false);
    static List<object> _workingList = new List<object>();
    static void DoWork()
    {
        Thread tradeThread1 = new Thread(() =>
        {
            Thread.CurrentThread.IsBackground = true;
            string tradeMessage = string.Empty;

            while (true)
            {
                Monitor.Enter(_syncO);
                var arr = _workingList.ToArray();
                Monitor.Exit(_syncO);

                // race condition when the set happens just before the 
                // thread was locked
                if (arr.Count() == 0)
                    _signal.WaitOne();

                Monitor.Enter(_syncO);
                arr = _workingList.ToArray();
                Monitor.Exit(_syncO);

                int count = 0;
                var deleteList = new List<object>();
                while (true)
                {
                    foreach (var item in arr)
                    {
                        // the value is changing every iteration. 
                       // this is why I need the Sleep at the end
                        bool b = Handle4(item);
                        if (b)
                            deleteList.Add(item);
                    }

                    if (count == 100) 
                        break;

                    Thread.Sleep(100);
                    count++;
                }

                // remove done tasks from _workingList
                RemoveItems(deleteList);                    

                // we can't close, so we re-set footprints(notifications) on our price cable. -> execute on broker tick(the right tick)
                // reHang trade on cable
                foreach (var item in _workingList)
                {
                    // re-use the undeleted tasks
                }

            }
        });

        tradeThread1.Start();
    }

Based on the help of @John Wu I come up with the following solution: The BlockingCollection will act as gate for the working thread. each iteration will copy the new task

private static void AddWork(object tap)
    {
        queue.Add(tap);
    }

    private static BlockingCollection<object> queue = new BlockingCollection<object>();

    static void Work()
    {
        Thread tradeThread1 = new Thread(() =>
        {                
            while (true)
            {
                var workingList = new List<object>();
                var deleteList = new List<object>();
                var reEvaluateList = new List<object>();
                while (true)
                {
                    if (workingList.Count() == 0)
                    {
                        // thread will wait until new work arrives -> it will start working again on the first task to come in.
                        workingList.Add(queue.Take());
                    }

                    foreach (var item in workingList)
                    {
                        bool b = Handle4(item);
                        if (b)
                            deleteList.Add(item);
                        else
                            item.ExitCounter++;

                        if (item.ExitCounter == 1000)
                            reEvaluateList.Add(item);
                    }

                    RemoveItems(deleteList, workingList);

                    // we can't close, so we re-set 
                    // we reevaluate tasks that are working for X amount of time and didn't finish
                    foreach (var item in reEvaluateList)
                        ReEvaluate(item);

                    RemoveItems(reEvaluateList, workingList);

                    // wait.. the item change-over-time, so a wait is a type of calculation.
                    Thread.Sleep(100);

                    // we want to avoid locking if we still have task to process                                                
                    if (queue.Count() == 0)
                        continue;

                    // add new work to local list
                    workingList.Add(queue.Take());
                }
            }
        });

        tradeThread1.Start();
    }

It feels a-little-bit messy. Any ideas on how to make it better?

Sahelanthropus
  • 164
  • 1
  • 4
  • 13
  • My recommendation is to use the producer-consumer pattern and C#'s [BlockingCollection](https://stackoverflow.com/questions/6512033/classic-producer-consumer-pattern-using-blockingcollection-and-tasks-net-4-tpl) to store pending tasks. If you do it that way, you don't have to deal with any locking primitives. – John Wu Feb 12 '19 at 20:41
  • All tasks should be evaluated at the same iteration.. (to do all the tasks at once, not one by one) – Sahelanthropus Feb 12 '19 at 20:43
  • 1
    "3. If more tasks are added while the working thread is active, it should complete them too." Sounds like there is only one thread and many tasks, so they are going to get done "one by one" as you put it. Do you wish to revise your question to allow multiple threads? – John Wu Feb 12 '19 at 21:32
  • How did you detect the race conditions and why don't you want to use the standard things (like suggested BlockingCollection) for the tasks queue ? – Dmytro Mukalov Feb 13 '19 at 06:22
  • @John Wu No still a single thread. This single thread should do-them one-by-one(tasks are running for predefined time(as a bulk)). tasks should run one-by-one as whole list -> once I got my list. I'm working on it. the BlockingCollection collection give my one at the time -> so each time a task will added it signals -> I Only want One signal -> if other tasks are added while the working thread is running, No signal is needed. – Sahelanthropus Feb 13 '19 at 10:21
  • @Dmytro Mukalov I can see the thread waiting in WaitOne While the List has updated. – Sahelanthropus Feb 13 '19 at 16:19
  • @Universe68 Not sure I understand why you want to do that, but I guess you could use a `BlockingCollection>` so the producer can add several items at once with only one signal.. – John Wu Feb 13 '19 at 16:47
  • But I don't see any code which would make `_signal.Set` to allow the thread go further, so this is not a race condition for me. And question again why don't you consider to use standard .NET classes for that? – Dmytro Mukalov Feb 13 '19 at 17:14
  • @Dmytro Mukalov add task can be done from any place in the system. so set can by done any time and in any place – Sahelanthropus Feb 13 '19 at 17:24
  • Under what circumstances would you expect the thread quits from `_signal.WaitOne()` ? – Dmytro Mukalov Feb 13 '19 at 17:27
  • @Dmytro Mukalov there is a public static function that has the add(locked) to task list and the _signal.Set() in it – Sahelanthropus Feb 13 '19 at 19:22
  • So how can we guess what's going with code if you didn't post the pieces which according to what you said have direct influence on the behavior? – Dmytro Mukalov Feb 13 '19 at 19:28
  • @Dmytro Mukalov OK you are right -> I added.. it's pretty straightforward -> the code can be the problem, but i think the way I deal with the problem is wrong. – Sahelanthropus Feb 13 '19 at 19:40
  • @John Wu I have many producers but each one can add one at the time.. I want all the producers tasks will be joined to a single place, which will be handled by a single thread.. the problem starts when I want that Handling part to rest when no work is present, and I have a race condition at the resting part. – Sahelanthropus Feb 13 '19 at 19:46
  • @John Wu the calculations are then multi-threaded but the handling of that calculation is done in a single thread -> so basically I want to make a transition from multi-threaded to a single thread system that handle all the multi-threaded tasks – Sahelanthropus Feb 13 '19 at 19:55
  • OP, I'm very confused now. You wanted the producer to be able to add groups of items at a time. Now you are saying that there are several producers adding items one at time. If that is the case, what constitutes a group? – John Wu Feb 13 '19 at 20:09
  • @John Wu "You wanted the producer to be able to add groups of items" I didn't say it -> All added tasks become a group that a single thread is handling – Sahelanthropus Feb 13 '19 at 20:13
  • OP, given the requirement "add individual items that become a group," can you define what constitutes a "group"? For example, when a producer thread adds an item, how does the consumer thread know whether the new item is the last item of a new group, so it can start processing? – John Wu Feb 13 '19 at 22:36
  • @John Wu group can also be a single item.. I would like for the thread to process any existing items. -> then after X amount of time the thread(our single handling thread) will remove the items. and it will do it until the task list is empty -> and When the list is empty it should rest. It will start again(Signal to wake-up) when the first item will be added to the group. – Sahelanthropus Feb 14 '19 at 10:59
  • @John Wu I made a solution based on your help -> can you please re-view-it ? :) – Sahelanthropus Feb 14 '19 at 11:39

0 Answers0