-1

I'm currently working on a multithread project involving several time consuming tasks running in parallel.

Every task is logging in the same file, and I need to keep it tidy (with the right order), so I found a way to log "task by task":

  • When a task must write to the log file, instead I write my logs in a Dictionary : _logList[id].Add(message)
  • This Dictionary has Key = ThreadID, Value = logs of the worker with this ID (list of string)
  • The ThreadID comes from Thread.CurrentThread.ManagedThreadId

Here is how I launch my BackgroundWorkers:

foreach (MyTask task in List<MyTask> tasks)
{
   BackgroundWorker bg = new BackgroundWorker();
   bg.DoWork += bgworker_DoWork;
   bg.RunWorkerCompleted += bgworker_RunWorkerCompleted;
   List<object> arguments = new List<object>() { task, args1 };
   bg.RunWorkerAsync(arguments);
}

The bgworker_DoWork delegate is the time consuming tasks, writing logs.

Then, when a task is over, the BackgroundWorker runs the delegate bgworker_RunWorkerCompleted, which basically ends the task and writes the corresponding log, in the right order:

Log.FlushWrite((int)result[2]); -- result[2] contains the threadId to flush

And FlushWrite only writes to the log file the list of logs of the threadId:

WriteMessageToFile(_logList[id]);

Most the time, this works. But sometimes, I see a weird behavior:

  • Several tasks are using the same ThreadID
  • When the first task N of the ThreadID X has ended, it calls the RunWorkerCompleted delegate, and flushes its logs + only a part of the next task N+1 of the ThreadID X (because the task has not ended yet)
  • When the task N+1 really ends, it flushes the remaining logs to write, but the log file is a mess because it is not tidy!

I know a managed thread id might be recycled and reused, but imo the only way for this situation to happen is that the thread is already reused (new call to DoWork) when the BackgroundWorker running has not yet called the RunWorkerCompleted

Am I right? Is there something I did not understand of this situation?

Maybe using a ManagedThreadId as a key of a Dictionary is a bad practice in that case?

Thanks for your help :)

Tjamat
  • 193
  • 1
  • 11
  • 1
    Indeed, why use ManagedThreadId as a key (only with weak assumption of its uniqueness) when you [probably] have unique tasks? Why not use MyTask as the dictionary key? But still you can do better and pass a logger to the gbWorker_doWork. And finally, why use BackgroundWorker? Are you developing a winforms app? – Alexey Adadurov Nov 20 '20 at 17:57
  • I tought using the ManagedThreadId was the simplest key. Indeed my tasks are unique, so I could use it as key! What do you mean by passing the logger, how could that do work? And finally I only know multithreading with BackgroundWorker: I'm not developing a winform app, do you think I should not use this class for my tasks? What do you suggest me? :) Thanks – Tjamat Nov 20 '20 at 20:25
  • Instead of the `BackgroundWorker` I would suggest to use `Task.Run` and async/await, combined with a `Progress` if you need to report progress. You can see [here](https://stackoverflow.com/questions/12414601/async-await-vs-backgroundworker/64620920#64620920) a practical comparison between the two options. The only justification for using `BackgroundWorker`s today for new development is when maintaining legacy apps, that run on out of date versions of .NET Framework. – Theodor Zoulias Nov 20 '20 at 23:27
  • @TheodorZoulias thanks for your answer. I cant change to Task.Run but do you think it will solve my logging problem if I still use the ManagedThreadId as a key? Or do you mean that I should use the Progress to write the logs? – Tjamat Nov 23 '20 at 07:53
  • Sorry, your problem is too complicated for me to understand, so I can't really offer any suggestion about how to solve it. I just mentioned the `Task.Run` and async/await as a modern substitute of the obsolete `BackgroundWorker`. If you can't use `Task.Run`, then the value of using a `Progress` is severally diminished. The `BackgroundWorker` is equipped with its own progress-reporting mechanism, so why mix it? – Theodor Zoulias Nov 23 '20 at 15:37

1 Answers1

1

I found my problem: during the FlushWrite operation, a lock was performed. If the worker was waiting for too long, another worker could write to the same ThreadID, thus the flush operated in the case was inconsistant.

Solved by using another dictionary, used only for flushing "complete workers".

The code has also been rewritten to change BackGroundWorker for Task.Run. Thanks :)

Tjamat
  • 193
  • 1
  • 11