CODEWITHSUNDEEP
c#multithreadingtask-parallel-library
rasx
rasx

Reputation: 5338

Is wrapping a Task with lock not very useful?

What intent is expressed here?:

lock(Locker)
{
    Task.Factory.StartNew(()=>
    {
        foreach(var item in this.MyNonCurrentCollection)
        {
          //modify non-concurrent collection
        }
    }, CancellationToken.None, TaskCreationOptions.None, TaskScheduler.FromCurrentSynchonizationContext())
    .ContinueWith(t => this.RaisePropertyChanged("MyNonCurrentCollection"));
}

Will the system lock (queue) until the Task completes or will the system lock only to start a new Task? The latter implies that this lock is kind if useless, right? I am just trying to discover intent from someone else's code. The ideal here is to protect MyNonCurrentCollection from being modified by another thread.

Upvotes: 3

Views: 985

Answers (2)

Michael J. Gray
Michael J. Gray

Reputation: 9896

The system will lock until the task is instantiated and kicked off. Task.Factory.StartNew is asynchronous. Your lock should not be acquired for very long, even if the task takes a while.

Inside the task, you should be actually locking the shared resource, not around the creation of the task. The lock will not have an effect on the safety of the resource unless the task completes extremely quickly and gets preemptively scheduled before the lock is exited.

This is a bug, yes.

Upvotes: 2

Servy
Servy

Reputation: 203817

Will the system lock (queue) until the Task completes

No.

will the system lock only to start a new Task?

Yes.

The latter implies that this lock is kind if useless, right?

It would seem so, although you can't always be sure without seeing the full context. For example, sometimes I'll write code that needs to check if it should start a task, based on a resource that requires locking, thus locking around code that just starts the task might be appropriate. If you're not doing anything besides starting the task though, that's probably not the case.

The ideal here is to protect MyNonCurrentCollection from being modified by another thread.

This does nothing to prevent that.

Side note, modifying a collection inside of a foreach over that collection is a bad idea. Some collections will be nice enough to just throw some sort of concurrent modification exception. Less nice collections will just produce mangled results.

Upvotes: 7

Related Questions