同期オブジェクトのスコープは安全か

いや、特に .NET に特化した話題じゃなくて、Java なんかでもいえることなのですが、サンプルなんかでもよく this を同期オブジェクトにしているんですよね。

private object cache = null;

public object Method()
{
  if (this.cache == null)
  {
    lock(this)
    {
      if (this.cache == null)
      {
        this.cache = new xxx(...);
        this.cache.SomeProperty = ...;
      }
    }
  }

  return this.cache;
}

みたいなかんじで、よくあるサンプルかもしれませんが、this なんていう非常に published なものを lock() の引数にしちゃうのは、非常に危険なことです。private なクラスを private なメンバにもっているとか、そういう場合もあるでしょうけど、this に相当するオブジェクトは、どこかからアクセス可能である以上は、そこから lock() の対象にすることが可能なわけで、このメソッドやこのクラスの他のメソッドだけが占有的に利用できる同期オブジェクトではないのです。
適当に object を１つ作ればいいんです、多少のメモリを犠牲にしますが、それは安全性と比べれば比較にならないぐらい小さいものでしょう。

private object synchronizer = new object();
private object cache = null;

public object Method()
{
  if (this.cache == null)
  {
    lock(this.synchronizer)
    {
      if (this.cache == null)
      {
        this.cache = new xxx(...);
        this.cache.SomeProperty = ...;
      }
    }
  }

  return this.cache;
}

実は、これでもまだダメです・・・というか、上のように修正をかけたときに気が付かず、やっちゃいました（汗
this.cache に代入してから this.cache.SomeProperty を設定する前に別スレッドがこのメソッドを呼び出したらどうなるか？ ということが考慮されていなかったのです。

  if (this.cache == null)
  {
    xxx x = new xxx(...);
    x.SomeProperty = ...;

    this.cache = x;
  }

たとえば、こんな感じにしてあげないとダメですね。