2009-12-03 5 views
4

Vous recherchez simplement une révision de code de ce code. ASP.net Cache n'est pas une option. La liste statique sera beaucoup consultée sur un site Web qui affiche bien plus de 10 000 pages vues par jour et des tentatives de lectures simultanées sont probables. Lors du redémarrage de l'application lorsque la liste est reconstruite, je me demandais s'il y avait des problèmes que je pourrais négliger? Est-ce que le verrouillage sur la liste est une bonne pratique instanciée?C# Verrouillage de propriété statique

public class MyClass 
{ 
     private static List<Entry> _listCache = null; 
     protected static List<Entry> ListCache 
     { 
      get 
      { 

       if (_listCache == null) 
       { 
        _listCache = new List<Entry>(); 
        lock (_listCache) 
        { 
         //Add items to the list _listCache from XML file 
        } 
       } 
       return _listCache; 
      } 
     } 
     //....Other methods that work with the list 
} 

Répondre

1

Plusieurs threads peuvent _listCache initialiser. En fonction des optimisations de génération de code et de l'optimisation de l'exécution à l'exécution, plusieurs threads peuvent être verrouillés et mettre à jour différents objets. Et d'ailleurs, vous ne pouvez pas exposer la liste comme une propriété permettant à quiconque d'ajouter/supprimer des objets sans un verrou.

Vous feriez mieux utiliser une liste immuable que plusieurs lecteurs peuvent analyser en toute sécurité en mode lecture seule. Vous pouvez également utiliser un verrou en lecture-écriture, mais les choses deviendront plutôt difficiles, entre le contrôle d'initialisation et le contrôle d'accès r-w.

12

10k vues - c'est un toutes les 8 secondes ... pas sûr que vous devez vous inquiétez pas trop ... ;-P

Mais re le code - qui est des choses trop compliquer, et vous pourriez encore jusqu'à la fin de l'initialisation deux fois. J'utiliserais simplement un constructeur statique pour faire ceci; ça va être plus robuste. Si vous doit avoir plein isolé chargement paresseux (même avec d'autres méthodes statiques sur le type), il y a un truc avec une classe interne pour obtenir le même:

public class MyClass 
{ 
    static class InnerCache { 
     internal static readonly IList<Entry> _listCache; 
     static InnerCache() { 
      List<Entry> tmp = new List<Entry>(); 
      //Add items to the list _listCache from XML file 
      _listCache = new ReadOnlyCollection<Entry>(tmp); 
     } 
    } 
    protected static IList<Entry> ListCache { 
     get {return InnerCache._listCache;} 
    } 
} 

Je serais aussi préoccupé par la possibilité de Quelqu'un mutant la liste - pourrait vouloir utiliser une liste en lecture seule!

+6

+1. Être pédant: 10k vues - c'est un toutes les 8 secondes * en moyenne *. Selon la loi de Murphy, les deux premiers se produiront au même moment. –

+1

Ou tout simplement l'initialiser dans l'événement Application_Start –

+3

En d'autres termes, si la probabilité d'une collision est de 0, alors il est sacrément près de 1. –

2

Un constructeur statique peut être votre meilleur pari ici. Un constructeur statique bloquera tous les threads qui en dépendent pendant qu'il s'exécute, et il ne fonctionnera qu'une fois. Comme vous avez le code ici, le verrou ne fait vraiment rien, et il y a beaucoup de façons que de mauvaises choses peuvent arriver, y compris plusieurs listes initialisées à partir de XML en même temps. En fait, un thread peut créer une nouvelle liste puis verrouiller et charger une liste différente puis retourner une troisième liste, en fonction du moment où la commutation de thread se produit.

+0

En fait, maintenant que je le regarde à nouveau, il pourrait même charger une liste différente de celle qui était verrouillée, et si le chargement se passe sur la variable statique, il pourrait charger différentes portions de plusieurs listes! –

2

Il n'y a pas vraiment une raison pour laquelle cela ne fonctionnerait pas pour vous. Cependant, si vous voulez le faire comme votre exemple de code, vous voulez verrouiller avant de vérifier si _listCache est null. Donc, vous auriez besoin d'un moniteur séparé pour verrouiller. Quelque chose comme ceci:

public class MyClass 
{ 
     private static object _listCacheMonitor = new object(); 
     private static List<Entry> _listCache = null; 
     protected static List<Entry> ListCache 
     { 
      get 
      { 
       lock (_listCacheMonitor) {  
        if (_listCache == null) 
        { 
         _listCache = new List<Entry>(); 
         //Add items to the list _listCache from XML file 
        } 
       } 
       return _listCache; 
      } 
     } 
     //....Other methods that work with the list 
}