views:

94

answers:

4
+2  Q: 

C# Multithreading

Okay. I want to have two threads running. Current code:

    public void foo()
    {      
            lock(this)
            {
                while (stopThreads == false)
                {
                   foreach (var acc in myList)
                   {
                     // process some stuff
                   }
                 }
             }
     }

    public void bar()
    {      
            lock(this)
            {
                while (stopThreads == false)
                {
                   foreach (var acc in myList)
                   {
                     // process some stuff
                   }
                 }
             }
     }

Both are accessing the same List, the problem is that the first thread "foo" is not releasing the lock i guess; because "bar" only starts when "foo" is done. Thanks

+3  A: 

Yes, that's how lock is designed to work.

The lock keyword marks a statement block as a critical section by obtaining the mutual-exclusion lock for a given object, executing a statement, and then releasing the lock.

Mutual-exclusion means that there can be at most one thread that holds the lock at any time.

Locking on this is a bad idea and is discouraged. You should create a private object and lock on that instead. To solve your problem you could lock on two different objects.

private object lockObject1 = new object();
private object lockObject2 = new object();

public void foo()
{      
    lock (lockObject1)
    {
         // ...
    }
}

public void bar()
{      
    lock (lockObject2)
    {
         // ...
    }
}

Alternatively you could reuse the same lock but move it inside the loop so that each loop has a chance to proceed:

while (stopThreads == false)
{
   foreach (var acc in myList)
   {
       lock (lockObject)
       {
           // process some stuff
       }
   }
}

However I would suggest that you spend some time to understand what is going on rather than reordering the lines of code until it appears to work on your machine. Writing correct multithreaded code is difficult.

For stopping a thread I would recommend this article:

Mark Byers
What about stopThreads variable , it is not thread safe, may be he got totally wrong result.
saurabh
@saurabh: It could be made volatile. Or it could be made into a thread safe property.
Mark Byers
thanks, that worked.
foobar
A: 

You need to protect myList and stopThreads variable .

Obtaining lock on this keyword is not recommended even though it works, because you are locking on whole current class instance.

Try to obtain lock on different objects in different methods.

saurabh
A: 

The problem you have is that you work with a very coarse lock. Both Foo and Bad basically do not work concurrently because whoever starts first stops the other one for the COMPLETE WORK CYCLE.

It should, though, ONLY lock WHILE IT TAKES THINGS OUT OF THE LIST. Foreach does not work here - per definition. You ahve to put up a second list and have each thread REMOVE THE TOP ITEM (while lockin), then work on it.

Basically:

  • Foreach does not work, as both threads will run through the compelte list
  • Second, locks must be granular in that they only lock while needed.

In your case, you lock in foo will only be released when foo is finished.

TomTom
A: 

Since you are not really asking a question, I suggest you should read a tutorial on how threading works. A .Net specific guide can be found here. It features the topics "Getting Started", "Basic Synchronization", "Using Threads", "Advanced Threading" and "Parallel Programming".

Also, you are locking on "this". The Msdn says:

In general, avoid locking on a public type, or instances beyond your code's control. The common constructs lock (this), lock (typeof (MyType)), and lock ("myLock") violate this guideline:

  • lock (this) is a problem if the instance can be accessed publicly.

  • lock (typeof (MyType)) is a problem if MyType is publicly accessible.

  • lock(“myLock”) is a problem because any other code in the process using the same string, will share the same lock.

Best practice is to define a private object to lock on, or a private static object variable to protect data common to all instances.

tobsen