Why is lock(this) {…} bad?
https://stackoverflow.com/questions/251391/why-is-lockthis-bad
It is bad form to use this in lock statements because it is generally out of your control who else might be locking on that object.
In order to properly plan parallel operations, special care should be taken to consider possible deadlock situations, and having an unknown number of lock entry points hinders this. For example, any one with a reference to the object can lock on it without the object designer/creator knowing about it. This increases the complexity of multi-threaded solutions and might affect their correctness.
A private field is usually a better option as the compiler will enforce access restrictions to it, and it will encapsulate the locking mechanism. Using this violates encapsulation by exposing part of your locking implementation to the public. It is also not clear that you will be acquiring a lock on this unless it has been documented. Even then, relying on documentation to prevent a problem is sub-optimal.
Finally, there is the common misconception that lock(this) actually modifies the object passed as a parameter, and in some way makes it read-only or inaccessible. This is false. The object passed as a parameter to lock merely serves as a key. If a lock is already being held on that key, the lock cannot be made; otherwise, the lock is allowed.
This is why it's bad to use strings as the keys in lock statements, since they are immutable and are shared/accessible across parts of the application. You should use a private variable instead, an Object instance will do nicely.
Run the following C# code as an example.
using Xunit.Abstractions; using System; using System.Threading; using Xunit; using Xunit.Abstractions; namespace AssemblyTest { public class TestBase { protected readonly ITestOutputHelper Output; public TestBase(ITestOutputHelper tempOutput) { Output = tempOutput; } } } public class Person { public int Age { get; set; } public string Name { get; set; } public void LockThis() { lock (this) { System.Threading.Thread.Sleep(10000); } } } public class LockTest : TestBase { public LockTest(ITestOutputHelper tempOutput) : base(tempOutput) { } [Fact] public void Test() { var nancy = new Person {Name = "Nancy Drew", Age = 15}; var a = new Thread(nancy.LockThis); a.Start(); var b = new Thread(Timewarp); b.Start(nancy); Thread.Sleep(10); var anotherNancy = new Person {Name = "Nancy Drew", Age = 50}; var c = new Thread(NameChange); c.Start(anotherNancy); a.Join(); } private void Timewarp(object subject) { var person = subject as Person; if (person == null) throw new ArgumentNullException("subject"); // A lock does not make the object read-only. lock (person.Name) { while (person.Age <= 23) { // There will be a lock on 'person' due to the LockThis method running in another thread if (Monitor.TryEnter(person, 10) == false) { Output.WriteLine("'this' person is locked!"); } else Monitor.Exit(person); person.Age++; if (person.Age == 18) { // Changing the 'person.Name' value doesn't change the lock... person.Name = "Nancy Smith"; } Output.WriteLine("{0} is {1} years old.", person.Name, person.Age); } } } private void NameChange(object subject) { var person = subject as Person; if (person == null) throw new ArgumentNullException("subject"); // You should avoid locking on strings, since they are immutable. if (Monitor.TryEnter(person.Name, 30) == false) { Output.WriteLine( "Failed to obtain lock on 50 year old Nancy, because Timewarp(object) locked on string "Nancy Drew"."); } else Monitor.Exit(person.Name); if (Monitor.TryEnter("Nancy Drew", 30) == false) { Output.WriteLine( "Failed to obtain lock using 'Nancy Drew' literal, locked by 'person.Name' since both are the same object thanks to inlining!"); } else Monitor.Exit("Nancy Drew"); if (Monitor.TryEnter(person.Name, 10000)) { string oldName = person.Name; person.Name = "Nancy Callahan"; Output.WriteLine("Name changed from '{0}' to '{1}'.", oldName, person.Name); } else Monitor.Exit(person.Name); } }
Why does the lock object have to be static?
回答1
It isn't "very common to use a private static readonly object for locking in multi threading" - rather, it is common to use a lock at the appropriate / chosen granularity. Sometimes that is static. More often, IMO, it isn't - but is instance based.
The main time you see a static lock is for a global cache, or for deferred loading of global data / singletons. And in the latter, there are better ways of doing it anyway.
So it really depends: how is Locker used in your scenario? Is it protecting something that is itself static? If so, the lock should be static. If it is protecting something that is instance based, then IMO the lock should also be instance based.
回答2
It doesn't have to be static, in fact sometimes it should not be static.
The variable should live in the same scope as the methods where you use it for locking. If the methods are static, the variable should be static, and if the methods are instance methods, the variable should be an instance varible.
A static variable will still work when used to lock in an instance method, but then you will be locking too much. You will lock all methods in all instances, not just the methods in the same instance.
lock statement (C# Reference)
https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/lock-statement
The lock statement acquires the mutual-exclusion lock for a given object, executes a statement block, and then releases the lock. While a lock is held, the thread that holds the lock can again acquire and release the lock. Any other thread is blocked from acquiring the lock and waits until the lock is released.
Locking
http://www.albahari.com/threading/part2.aspx#_Locking
C#’s lock statement is in fact a syntactic shortcut for a call to the methods Monitor.Enter and Monitor.Exit, with a try/finally block. Here’s (a simplified version of) what’s actually happening within the Go method of the preceding example:
Monitor.Enter (_locker); try { if (_val2 != 0) Console.WriteLine (_val1 / _val2); _val2 = 0; } finally { Monitor.Exit (_locker); }
Calling Monitor.Exit without first calling Monitor.Enter on the same object throws an exception.
Why doesn't Lock'ing on same object cause a deadlock? [duplicate]
回答1
For the same thread a lock is always reentrant, so the thread can lock an object as often as it wants.
回答2
One word: Reentrant lock.
If a thread has already acquired a lock, then it does not wait if it wants to acquire the lock again. This is very much needed otherwise it could have turned simple recursive functions into a nightmare.!
Why do nested locks not cause a deadlock?
回答1
If a thread already holds a lock, then it can "take that lock" again without issue.
As to why that is, (and why it's a good idea), consider the following situation, where we have a defined lock ordering elsewhere in the program of a -> b:
void f()
{
lock(a)
{ /* do stuff inside a */ }
}
void doStuff()
{
lock(b)
{
//do stuff inside b, that involves leaving b in an inconsistent state
f();
//do more stuff inside b so that its consistent again
}
}Whoops, we just violated our lock ordering and have a potential deadlock on our hands.
doStuff先获得a的lock，然后f里面再去获得a的lock，就是一个可重入的锁
We really need to be able to do the following:
function doStuff()
{
lock(a)
lock(b)
{
//do stuff inside b, that involves leaving b in an inconsistent state
f();
//do more stuff inside b so that its consistent again
}
}So that our lock ordering is maintained, without self-deadlocking when we call f().
回答2
The lock keyword uses a re-entrant lock, meaning the current thread already has the lock so it doesn't try to reacquire it.
A deadlock occurs if
Thread 1 acquires lock A
Thread 2 acquires lock B
Thread 1 tries to acquire lock B (waits for Thread 2 to be done with it)
Thread 2 tries to acquire lock A (waits for Thread 1 to be done with it)
Both threads are now waiting on each other and thus deadlocked.
Re-entrant locks in C#
问题
Will the following code result in a deadlock using C# on .NET?
class MyClass
{
private object lockObj = new object();
public void Foo()
{
lock(lockObj)
{
Bar();
}
}
public void Bar()
{
lock(lockObj)
{
// Do something
}
}
}回答
No, not as long as you are locking on the same object. The recursive code effectively already has the lock and so can continue unhindered.
lock(object) {...} is shorthand for using the Monitor class. As Marc points out, Monitor allows re-entrancy, so repeated attempts to lock on an object on which the current thread already has a lock will work just fine.
If you start locking on different objects, that's when you have to be careful. Pay particular attention to:
- Always acquire locks on a given number of objects in the same sequence.
- Always release locks in the reverse sequence to how you acquire them.
If you break either of these rules you're pretty much guaranteed to get deadlock issues at some point.
Here is one good webpage describing thread synchronisation in .NET: http://dotnetdebug.net/2005/07/20/monitor-class-avoiding-deadlocks/
Also, lock on as few objects at a time as possible. Consider applying coarse-grained locks where possible. The idea being that if you can write your code such that there is an object graph and you can acquire locks on the root of that object graph, then do so. This means you have one lock on that root object and therefore don't have to worry so much about the sequence in which you acquire/release locks.
(One further note, your example isn't technically recursive. For it to be recursive, Bar() would have to call itself, typically as part of an iteration.)