thread safety

CodingYoshi

I saw this code

public class MyClass
{
protected static string a = "a";
public void DoStuff()
{
lock (a)
{
// Do things
}
}
}

why not

public class MyClass
{
public void DoStuff()
{
lock (this)
{
// Do things
}
}
}

CodingYoshi Artificial Intelligence is no match for Human Stupidity.

SledgeHammer01

Why would you lock an entire class when you are just working on a single object inside of the class? What if somebody else wants to work on a different part of it?

Luc Pattyn

There is only one a inside that class, however there could be many instances of that class; lock(a) is a lock shared by all instances, lock(this) isn't. Which one it should be depends on the data used inside the lock, is it instance-specific or is it global? :)

Luc Pattyn [My Articles] Nil Volentibus Arduum

AspDotNetDev

Because you don't want to expose a public member that other classes can put a lock on. It could lead to a deadlock, as with this example (you will never see "Done"):

public class MyClass
{
private Action<MyClass> action;
public MyClass(Action<MyClass> action)
{
this.action = action;
}
public void DoStuff()
{
MessageBox.Show("Started");
lock (this) { action(this); }
MessageBox.Show("Done");
}
}

// Somewhere else...
(new MyClass(delegate(MyClass instance) {
var t = new System.Threading.Thread(delegate()
{
lock (instance) { }
});
t.Start();
t.Join();
})).DoStuff();

Martin Fowler wrote:

Any fool can write code that a computer can understand. Good programmers write code that humans can understand.

AspDotNetDev

Good answer. I missed the fact that a is static.

Martin Fowler wrote:

Any fool can write code that a computer can understand. Good programmers write code that humans can understand.

CodingYoshi

What if the method is changing a data member which is also used in another method? I guess it will be locked for that method as well, right? But why a static variable? I looked at the class and looks like it is only used for this purpose CodingYoshi Artificial Intelligence is no match for Human Stupidity.

CodingYoshi

Ok so lock (this) means lock the calling object for the duration of the code block. lock(a) means lock a for the duration of the code block which means other threads still have access to the object just not a, am i right?

CodingYoshi Artificial Intelligence is no match for Human Stupidity.

Luc Pattyn

the terminology may be a little confusing, this is how I explain it: 1. every instance of lock(lock-object), all sharing the same lock-object, restricts access to whatever code is inside the lock's code block, in such a way that only one thread can get passed the lock-block's entry point. Once a thread has succeeded entering such code block, all other threads trying to enter the same lock-block (or some other lock-block on the same lock-object) will be stalled until the former leaves its lock-block, and only then one of the others is allowed to proceed. 2. So the lock-object is like a key that fits every lock statement that refers to it; the key is unique, there is only one of it (for a given lock-object), and initially it is available to everyone; the first thread to reach a lock statement grabs the key, and can proceed; he must relinquish the key when leaving the lock-block; in the mean time the other threads, encountering a lock, would have to wait for the key to become available. 3. locks only make sense when you have more than one thread (warning: a lot of asynchronous events, such as a network's DataReceived, would execute on different threads). 4. using locks correctly isn't easy for the novice; with too much locking, you may cripple the app's performance and the user experience, ultimately the app could come to a halt ("dead-lock"). With insufficient locking all may seem fine, until suddenly weird things start to happen, e.g. wrong results getting generated as some object gets into an unexpected state, when one thread changes one state variable, while another thread changes another, resulting in some inconsistencies. :)

Luc Pattyn [My Articles] Nil Volentibus Arduum

PIEBALDconsult

From the documentation[^]: " 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 since 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. "

CodingYoshi

Great and thanks for the explanation, definitely clears things up. I know how threads work but the part which confused me was the static part.

CodingYoshi Artificial Intelligence is no match for Human Stupidity.

Luc Pattyn

you're welcome. :)

Luc Pattyn [My Articles] Nil Volentibus Arduum

Lost User

Say the object has 2 methods that need locking but they can be independent.

DoStuff();//Needs it own lock
DoOtherStuff();//Needs it own lock

In this case you create a lock for each one (and please name appropriately)

object _lockStuff = new Object();
object _lockOtherStuff = new Object();

If you chose to use this as the locking object then the methods could not be run at the same time.

Computers have been intelligent for a long time now. It just so happens that the program writers are about as effective as a room full of monkeys trying to crank out a copy of Hamlet.

jschell

First don't use string literals as a locking mechanism. Basically it doesn't work the way you think it does (regardless of all other issues.) Second the semantics of your examples are different. In one case you are using a class level (static) and in the other you are using a instance level lock.

CodingYoshi

First don't use string literals as a locking mechanism. Basically it doesn't work the way you think it does Please elaborate and explain

CodingYoshi Artificial Intelligence is no match for Human Stupidity.

jschell

String literals are global to the application. Thus the following represents one locking object instance regardless of the number of class instances for either class. (And the examples specifically do not use static.)

 class A
 {
       private String lockObject = "a";

       public void Doit()
       {
            lock(lockObject) {...}
       }
 }
 class B
 {
       private String lockObject = "a";

       public void Doit()
       {
            lock(lockObject) {...}
       }
 }