Search code examples
c#thread-safetyref

Is access to fields VIA `ref` parameters guarded by lock statements in the CALLED method?


Given,

private object _x;

private object LoadAndSet(ref object x) {
   // lock established over read and update
   lock (somePrivateObjectNotUsedElsewhereThatIsIrrelvantToTheQuestion) {
       if (x == null)
           x = Load();
       return x;
   }
}

Invoked as,

public object Load() {
    return LoadAndSet(ref _x);
}
  • Are the Read / Writes to the Field (_x) "passed by reference" covered under the atomicity/visibility guarantees of the lock?

That is, is the first code equivalent to the following where the field is used directly? (The assignment occurs directly, instead of via a ref parameter.)

private object _x;

private object LoadAndSetFieldDirectly() {
   // lock established over read and update
   lock (somePrivateObjectNotUsedElsewhereThatIsIrrelvantToTheQuestion) {
       if (_x == null)
           _x = Load();
       return _x;
   }
}

public object Load() {
    return LoadAndSetFieldDirectly();
}

I suspect this to be true due to the use of ldind.ref and stind.ref in the method's MSIL; however, the question is begging authoritative documentation/information on the thread-safety (or lack of) when writing such ref code.


Solution

  • The semantics of lock(lockObject) { statements } are:

    • No two distinct threads are ever in a region of locked code that was locked with the same lockObject instance. If one thread is in the region then it will either go into an infinite loop, complete normally, or throw. (Or, for advanced players, goes into a wait state via Monitor.Wait; this is not intended to be a tutorial on the correct use of the monitor object.) A second thread will not enter the protected region until control leaves the region.
    • Reads of variables in or after the lock will not be moved "backwards in time" to before the lock.
    • Writes of variables in or before the lock will not be moved "forwards in time" to after the lock.

    (This is a quick informal summary; for exact details of what the C# specification guarantees about reordering of reads and writes, see the specification.)

    Those semantics are enforced by the runtime regardless of whether the variables accessed in the lock body are fields, locals, normal formal parameters, ref/out formal parameters, array elements or pointer dereferences.

    That said, three things make me nervous about your code.

    First, it's unnecessary and suboptimal re-invention of an existing mechanism. If you want to implement lazy initialization, use Lazy<T>. Why roll your own and risk getting it wrong, when you can use code written by experts who already have eked out every bit of performance from it?

    Second, you have to ensure that every use of the field is under a lock, not just its initialization. Passing a field as a ref parameter makes an alias for the field, and now you have made the job of verifying that you got every usage of the field under the lock harder.

    Third, it seems like there is an opportunity for unnecessary contention here. What if two different fields are both initialized by this same code on two different threads? Now they are contending for the lock when they do not need to be.