is below class thread safe ? explain

Question

Can someone explain whether or not I can call the below class "thread safe"?

As far as I know, we can call something thread safe if we are not breaking existing functionality

Example:

public class BackgroundWorker
{
    private readonly IDictionary<string, RunningTask> _runningTasks = new ConcurrentDictionary<string, RunningTask>();

    /// <summary>
    /// Executes async job for the specified key, only one at a time.
    /// </summary>
    /// <param name="key"></param>
    public void Enqueue(string key)
    {
        if (_runningTasks.ContainsKey(key))
        {
            _runningTasks[key].Repeat = true;
            return;
        }

        _runningTasks[key] = new RunningTask();

        ExecuteTask(key);
    }

    private void ExecuteTask(string key)
    {
        Task.Run(() =>
        {
            // Do something

            if (_runningTasks[key].Repeat)
            {
                _runningTasks[key].Repeat = false;
                ExecuteTask(key);
                return;
            }

            _runningTasks.Remove(key);
        });
    }

    private class RunningTask
    {
        /// <summary>
        /// Flag to repeat a task after completion.
        /// </summary>
        public bool Repeat { get; set; }
    }
}

[Please format the code blocks with this](https://meta.stackoverflow.com/questions/251361/how-do-i-format-my-code-blocks). — user202729, Mar 04 '18 at 07:20
Being thread safe has nothing to do with "breaking existing functionality" — mason, Mar 05 '18 at 17:25

rahulaga-msft · Answer 1 · 2018-03-07T12:09:25.643

I don't think so because _runningTasks is shared object and your method Enqueue is writing on this shared object. For example its possible when one thread already executed line number y, another thread will evaluate condition check in line number x as true - which might not be intention.

public void Enqueue(string key)
{
    if (_runningTasks.ContainsKey(key)) /*say line no : x */
    {
        _runningTasks[key].Repeat = true;
        return;
    }

    _runningTasks[key] = new RunningTask(); /*say line no:y*/

    ExecuteTask(key);
}

Using ConcurrentDictionary will just ensure no two threads can read/write to/from the dictionary same time.

To your second point :

As far as I know, we can call something thread safe if we are not breaking existing functionality

No this is not the definition of thread safe (might be ok to say one of desirable outcome in multi threaded environment) I would recommend to read this post for official meaning rather.

Just to clarify: A `ConcurrentDictionary` does not make the class thread-safe. Still, a thread may be preempted between line x and line `_runningTasks[key].Repeat = true;` and another thread may remove the task from the dictionary. Hence, the second line may throw a `KeyNotFoundException`. — Kristof U., Mar 05 '18 at 08:43
Doesn't ConcurrentDictionary have an [`AddOrUpdate` method](https://msdn.microsoft.com/en-us/library/ee378665.aspx)? — Ben Voigt, Mar 05 '18 at 17:25

is below class thread safe ? explain

1 Answers1