Should a return statement be inside or outside a lock?
I would definitely put the return inside the lock. Otherwise you risk another thread entering the lock and modifying your variable before the return statement, therefore making the original caller receive a different value than expected.
Essentially, which-ever makes the code simpler. Single point of exit is a nice ideal, but I wouldn't bend the code out of shape just to achieve it... And if the alternative is declaring a local variable (outside the lock), initializing it (inside the lock) and then returning it (outside the lock), then I'd say that a simple "return foo" inside the lock is a lot simpler.
To show the difference in IL, lets code:
static class Program
{
static void Main() { }
static readonly object sync = new object();
static int GetValue() { return 5; }
static int ReturnInside()
{
lock (sync)
{
return GetValue();
}
}
static int ReturnOutside()
{
int val;
lock (sync)
{
val = GetValue();
}
return val;
}
}
(note that I'd happily argue that ReturnInside
is a simpler/cleaner bit of C#)
And look at the IL (release mode etc):
.method private hidebysig static int32 ReturnInside() cil managed
{
.maxstack 2
.locals init (
[0] int32 CS$1$0000,
[1] object CS$2$0001)
L_0000: ldsfld object Program::sync
L_0005: dup
L_0006: stloc.1
L_0007: call void [mscorlib]System.Threading.Monitor::Enter(object)
L_000c: call int32 Program::GetValue()
L_0011: stloc.0
L_0012: leave.s L_001b
L_0014: ldloc.1
L_0015: call void [mscorlib]System.Threading.Monitor::Exit(object)
L_001a: endfinally
L_001b: ldloc.0
L_001c: ret
.try L_000c to L_0014 finally handler L_0014 to L_001b
}
method private hidebysig static int32 ReturnOutside() cil managed
{
.maxstack 2
.locals init (
[0] int32 val,
[1] object CS$2$0000)
L_0000: ldsfld object Program::sync
L_0005: dup
L_0006: stloc.1
L_0007: call void [mscorlib]System.Threading.Monitor::Enter(object)
L_000c: call int32 Program::GetValue()
L_0011: stloc.0
L_0012: leave.s L_001b
L_0014: ldloc.1
L_0015: call void [mscorlib]System.Threading.Monitor::Exit(object)
L_001a: endfinally
L_001b: ldloc.0
L_001c: ret
.try L_000c to L_0014 finally handler L_0014 to L_001b
}
So at the IL level they are [give or take some names] identical (I learnt something ;-p).
As such, the only sensible comparison is the (highly subjective) law of local coding style... I prefer ReturnInside
for simplicity, but I wouldn't get excited about either.
It doesn't make any difference; they're both translated to the same thing by the compiler.
To clarify, either is effectively translated to something with the following semantics:
T myData;
Monitor.Enter(mutex)
try
{
myData= // something
}
finally
{
Monitor.Exit(mutex);
}
return myData;