ASP.NET/Static classe race condition?

Question

Ho un'applicazione ASP.NET con un sacco di contenuti dinamici. Il contenuto è lo stesso per tutti gli utenti che appartengono a un particolare client. Per ridurre il numero di colpi database necessarie per ogni richiesta, ho deciso di memorizzare nella cache i dati a livello di client. Ho creato una classe statica ( "ClientCache") per contenere i dati.
Il metodo più usato, spesso della classe è di gran lunga "GetClientData", che riporta un oggetto ClientData che contiene tutti i dati memorizzati per un particolare client. ClientData è caricato pigramente, però: se i dati cliente richiesto è già memorizzato nella cache, il chiamante riceve i dati memorizzati nella cache; in caso contrario, i dati vengono recuperati, aggiunto alla cache e poi restituita al chiamante.

Alla fine ho cominciato a ricevere crash intermittenti nel metodo GetClientData sulla linea in cui viene aggiunto l'oggetto ClientData alla cache. Ecco il corpo del metodo:

public static ClientData GetClientData(Guid fk_client)
{
    if (_clients == null)
        _clients = new Dictionary<Guid, ClientData>();

    ClientData client;
    if (_clients.ContainsKey(fk_client))
    {
        client = _clients[fk_client];
    }
    else
    {
        client = new ClientData(fk_client);
        _clients.Add(fk_client, client);
    }
    return client;
}

Il testo eccezione è sempre qualcosa come "Un oggetto con la stessa chiave esiste già." Naturalmente, ho cercato di scrivere il codice in modo che semplicemente non era possibile aggiungere un client alla cache se esisteva già.

A questo punto, sto sospettando che ho una condizione di competizione e il metodo viene eseguito due volte contemporaneamente, il che potrebbe spiegare come potrebbe andare in crash il codice. Quello che sto confuso su, però, è come il metodo potrebbe essere eseguito due volte contemporaneamente a tutti. Per quanto ne so, tutte le applicazioni ASP.NET campi sempre e solo una richiesta alla volta (è per questo che possiamo usare HttpContext.Current).

Quindi, è probabile che questo bug una condizione di competizione che richiederà blocca la messa in sezioni critiche? O mi manca un più evidente bug?

Answer 1

Se un'applicazione ASP.NET gestisce solo una richiesta alla volta tutti i siti ASP.NET sarebbero in guai seri. ASP.NET può elaborare decine alla volta (tipicamente 25 per core CPU).

Si consiglia di utilizzare ASP.NET cache invece di utilizzare il proprio dizionario per memorizzare l'oggetto. Le operazioni nella cache sono thread-safe.

Nota è necessario essere sicuri che l'operazione di lettura sull'oggetto di memorizzare nella cache sono threadsafe, purtroppo classe più NET semplicemente dichiarare l'istanza membri non sono threadsafe senza cercare di puntare qualsiasi che potrebbe essere.

Modifica :

Un commento a questa risposta afferma: -

  

Solo operazioni atomiche nella cache sono thread-safe. Se fai qualcosa come controllo
  se una chiave esiste e quindi inserirlo, che non è thread-safe e possono causare la voce per
  sovrascritto.

La sua pena sottolineare che se ci sentiamo abbiamo bisogno di fare una simile operazione atomica, allora la cache non è probabilmente il posto giusto per la risorsa.

Ho un po 'di codice che fa esattamente come il commento descrive. Tuttavia la risorsa viene stoccato sarà lo stesso in entrambi i luoghi. Quindi se un elemento esistente in rare occasioni viene sovrascritto solo il costo è che un thread inutilmente generato una risorsa. Il costo di questo raro evento è molto inferiore al costo di cercare di rendere l'operazione atomica ogni volta che un tentativo di accesso è fatto.

Answer 2

Questo è molto facile da risolvere:

private _clientsLock = new Object();

public static ClientData GetClientData(Guid fk_client)
{
  if (_clients == null)
    lock (_clientsLock)
      // Check again because another thread could have created a new 
      // dictionary in-between the lock and this check
      if (_clients == null) 
        _clients = new Dictionary<Guid, ClientData>();

  if (_clients.ContainsKey(fk_client))
    // Don't need a lock here UNLESS there are also deletes. If there are
    // deletes, then a lock like the one below (in the else) is necessary
    return _clients[fk_client];
  else
  {
    ClientData client = new ClientData(fk_client);

    lock (_clientsLock)
      // Again, check again because another thread could have added this
      // this ClientData between the last ContainsKey check and this add
      if (!clients.ContainsKey(fk_client))
       _clients.Add(fk_client, client);

    return client;
  }
}

Tenete a mente che ogni volta che si fa confusione con le classi statiche, si ha la possibilità di problemi di sincronizzazione thread. Se c'è un elenco a livello di classe statica di qualche tipo (in questo caso, _clients, l'oggetto Dictionary), c'è sicuramente sarà problemi di sincronizzazione filo da affrontare.

Answer 3

Il codice in realtà si assume solo un thread è in funzione alla volta.

Questo semplicemente non sarà vero in ASP.NET

Se ti ostini a fare in questo modo, utilizzare un semaforo statica per bloccare l'area intorno a questa classe.

Answer 4

è necessario thread-safe e ridurre al minimo serratura.
 vedi interblocco ricontrollato ( http://en.wikipedia.org/wiki/Double-checked_locking )

scrittura semplicemente con TryGetValue.

public static object lockClientsSingleton = new object();

public static ClientData GetClientData(Guid fk_client)
{
    if (_clients == null) {
        lock( lockClientsSingleton ) {
            if( _clients==null ) {
                _clients = new Dictionary``();
            }
        }
    }
    ClientData client;
    if( !_clients.TryGetValue( fk_client, out client ) )
    {
        lock(_clients) 
        {
            if( !_clients.TryGetValue( fk_client, out client ) ) 
            {
                client = new ClientData(fk_client)
                _clients.Add( fk_client, client );
            }
        }
    }
    return client;
}