以下のヘルパークラスは安全ですか？

Question

私はシーケンシャルIDを追跡する必要があります。これは、4つのテーブルにわたってmax（id）を実行しているSPを介して私に返されています。シーケンスを管理しているデータベースにはidentifer/sequenceはありません。これは明らかに並行性の問題があるので、私は一意のIDが常に生成されるようにヘルパークラスを作成しました。

ヘルパーは、最初に現在のIdを見つけるためにDBを呼び出すリポジトリを介して初期化され、その後のすべてのID要求はヘルパー経由でメモリ内で処理されます。 DB（鉱山）を使用しているアプリは1つしかないので、他の誰かが来て、トランザクションを作成することについて心配する必要はありません。& Idの同期が外れています。私は、IVEは、スレッドsafteyの基礎を得たと思う誰かが助言してくださいすることができます:)

private class TransactionIdProvider 
{ 
private static readonly object Accesslock = new object(); 
private int _transactionId; 
public int NextId 
{ 
    get 
    { 
     lock (Accesslock) 
     { 
      if(!Initialised) throw new Exception("Must Initialise with id first!!"); 
      return _transactionId++; 
     } 
    } 
} 

public bool Initialised { get; private set; } 

public void SetId(int id) 
{ 
    lock (Accesslock) 
    { 
     if (Initialised) return; 
     _transactionId = id; 
     Initialised = true; 
    } 
} 

public TransactionIdProvider() 
{ 
    Initialised = false; 
} 
} 

ヘルパークラスがリポジトリに初期化され、ヘルパーが初期化される競合状態が心配イム：

private static readonly TransactionIdProvider IdProvider = new TransactionIdProvider(); 

    public int GetNextTransactionId() 
    { 
     if(!IdProvider.Initialised) 
     { 
      // Ask the DB 
      int? id = _context.GetNextTransactionId().First(); 
      if (!id.HasValue) 
       throw new Exception("No transaction Id returned"); 
      IdProvider.SetId(id.Value); 
     } 

     return IdProvider.NextId; 
    } 

Answer 1

スレッドセーフですが、不必要に遅いです。
番号を増やすだけのロックは必要ありません。代わりに、あなたは原子数学を使うことができます。

また、すべてのインスタンス（これはstatic）でロックを共有していますが、これは不要です。 （2つの異なるインスタンスを一度に実行することで何も問題はありません）

最後に、（IMHO）別の初期化されていない状態が存在することはありません。

私はこのようにそれを記述します。

class TransactionIdProvider { 
    private int nextId; 
    public TransactionIdProvider(int id) { 
     nextId = value; 
    } 

    public int GetId() { 
     return Interlocked.Increment(ref nextId); 
    } 
} 

Answer 2

はい、それはスレッドセーフです。しかし、IMOのロックはグローバルすぎる - インスタンスデータを保護するための静的なロックが少し過剰な攻撃を襲う。

また、プロパティとしてのNextIdは状態が変わるため、メソッドにする必要があります。

ロックを介してInterlocked.Incrementを使用することもできますが、ほとんどのクラスが変更されます。

最後に、SetId - 既に初期化されている場合は、コールを無視して無視するのではなく、例外（InvalidOperationException）をスローします。これはエラーのようです。もちろん、それはInitializedとSetIdの呼び出しの間に厄介な間隔を導入します。SetIdはSetIdが変更を行った場合trueを返すことができ、setの時点で初期化されたと判明した場合はfalseですが、SLaksの方法はより良いです。

Answer 3

これは良い考えではないと思いますが、これに対処する別の方法があります。 通常、本当にユニークなIDが必要な場合、IDが使用されているかどうかを計算上有効な方法がない場合は、GUIDを使用します。

ただし、ロックする代わりにインターロックされた操作を使用することはできますが、ロックすることなくそのまま行うことができます。

Interlocked.Increment、Interlocked.ExchangeとInterlocked.CompareExchangeを探し

private class TransactionIdProvider 
{ 
    private volatile int _initialized; 
    private int _transactionId; 

    public int NextId 
    { 
     get 
     { 
      for (;;) 
      { 
       switch (_initialized) 
       { 
        case 0: throw new Exception("Not initialized"); 
        case 1: return Interlocked.Increment(ref _transactionId); 
        default: Thread.Yield(); 
       } 
      } 
     } 
    } 

    public void SetId(int id) 
    { 
     if (Interlocked.CompareExchange(ref _initialized, -1, 0) == 0) 
     { 
      Interlocked.Exchange(ref _transactionId, id); 
      Interlocked.Exchange(ref _initialized, 1); 
     } 
    } 
} 

これはあなたに警告を与えるだろうが、それは正常であり、また、法的などのC＃のドキュメントで報告されます。あなたがIsInitializedをチェックする必要がない場合は

// Disable warning "A reference to a volatile field will not be treated as volatile" 
#pragma warning disable 0420 

は、あなたが最も簡単な方法でそれを行うことができます：

public int NextId() 
{ 
    return Interlocked.Increment(ref _transactionId); 
} 

public void Set(int value) 
{ 
    Interlocked.Exchange(ref _transactionId, value); 
}