Reputation: 77
I have a private static field which I use for synchronization (lock). Now I have two functions which I don't want to execute concurrently. So I did this:
public class Synchronization
{
private static object _lock = new object();
public void MethodA()
{
lock (_lock)
{
Console.WriteLine("I shouldn't execute with MethodB");
}
}
public void MethodB()
{
lock (_lock)
{
Console.WriteLine("I shouldn't execute with MethodA");
}
}
}
I know that locking on an object will prevent parallel execution of a single function, but will the same work if I use the same lock object in different methods which run concurrently? Simply, put can any other thread acquire lock on an already locked object in another function?
Upvotes: 4
Views: 10472
Reputation: 38434
Only a single thread at a time may acquire the lock, so this state is exclusive for all threads on a single lock instance. So in your example, only one method body may be executing at any given time for all instances of class Synchronization
as your lock is static. If you want locking per instance of your class, then do not mark the lock object as static.
Your assumptions regarding synchronization are correct.
Please note that you should mark the lock object readonly
for a completely water-tight solution. As the code stands, it would be possible for the lock object to be re-assigned and so break the locking semantics, e.g.:
public class Synchronization
{
private static object _lock = new object();
public void MethodA()
{
lock (_lock)
{
Console.WriteLine("I shouldn't execute with MethodB");
}
}
public void MethodB()
{
//This shouldn't be allowed!
_lock = new object();
lock (_lock)
{
Console.WriteLine("I shouldn't execute with MethodA");
}
}
}
Lock object should be marked as readonly
, i.e.:
private static readonly object _lock = new object();
Upvotes: 5
Reputation: 7612
First, the _lock
should not be static. Or do you want multiple instances of the object to lock themselves each other? Second, you should have only one synchronized method in a class. Even more, you should avoid dependencies between synchronized methods in your class. Otherwise, you are in a risk that the caller of your methods will do it wrong and get unexpected behavior.
Consider, for example, this code:
class Synchronized
{
object lockObj = new object();
int counter = 100;
public void Decrement()
{
lock (this.lockObj)
{
this.counter--;
}
}
public int IsZero()
{
lock (this.lockObj)
{
return this.counter == 0;
}
}
}
Now what will one do with shared Synchronized instance?
Use it like this
while (!synchronized.IsZero())
{
synchronized.Decrement();
}
Now thread 1 calls Decrement, counter gets to 0 and immediately thread 2 calls Decrement, because it was waiting on lock in Decrement method, not IsZero method. Counter is now -1 and the loop is infinite.
It's not that the locking mechanism was incorrectly programmed, but that the caller didn't use it well. If you only exposed one synchronized method on your Synchronized class, you wouldn't fool the programmer to blindly believe it's safe.
It should be something like this:
class Synchronized
{
object lockObj = new object();
int counter = 100;
public bool IfNotZeroDecrement()
{
lock (this.lockObj)
{
if (this.counter > 0)
this.counter--;
return this.counter > 0;
}
}
}
/// Usage:
while (synchronized.IfZeroDecrement())
{
}
Upvotes: 1
Reputation: 53183
Locks are granted based on the object that is the target of the lock, not the method in which the lock
statement occurs. So in your case you multiple thread might enter the various methods but only one thread at a time will be able to execute any code that's within the lock
statement.
Upvotes: 1
Reputation: 52518
You are doing it correctly. You have created two critical sections that will not be entered on the same time.
So MethodA and MethodB will not be "active" at the same time. And also there will be only one MethodA (and MethodB) active at the same time.
This is valid for across all objects that you create. What I mean is: only one thread will be in any MethodA or MethodB from any object. If you want the lock to occur only within one object, you can make the _lock object not static.
Upvotes: 1