Minuterie et IDposable - Protection supplémentaire dans Dispose?

Question

J'ai une classe qui crée et utilise un System.Threading.Timer, quelque chose comme ceci:

using System.Threading; 

public class MyClass : IDisposable 
{ 
    private List<int> ints = new List<int>(); 

    private Timer t; 

    public MyClass() 
    { 
    //Create timer in disabled state 

    t = new Timer(this.OnTimer, null, Timeout.Infinite, Timeout.Infinite); 
    } 

    private void DisableTimer() 
    { 
    if (t == null) return; 

    t.Change(Timeout.Infinite, Timeout.Infinite); 
    } 

    private void EnableTimer() 
    { 
    if (t == null) return; 

    //Fire timer in 1 second and every second thereafter. 
    EnableTimer(1000, 1000); 
    } 

    private void EnableTimer(long remainingTime) 
    { 
    if (t == null) return; 

    t.Change(remainingTime, 1000); 
    } 

    private void OnTimer(object state) 
    { 
    lock (ints) //Added since original post 
    { 
     DisableTimer(); 

     DoSomethingWithTheInts(); 
     ints.Clear(); 

     // 
     //Don't reenable the timer here since ints is empty. No need for timer 
     //to fire until there is at least one element in the list. 
     // 
    } 
    } 

    public void Add(int i) 
    { 
    lock(ints) //Added since original post 
    { 
     DisableTimer(); 

     ints.Add(i); 
     if (ints.Count > 10) 
     { 
     DoSomethingWithTheInts(); 
     ints.Clear(); 
     } 

     if (ints.Count > 0) 
     { 
     EnableTimer(FigureOutHowMuchTimeIsLeft()); 
     } 
    } 
    } 

    bool disposed = false; 

    public void Dispose() 
    { 
    //Should I protect myself from the timer firing here? 

    Dispose(true); 
    } 

    protected virtual void Dispose(bool disposing) 
    { 
    //Should I protect myself from the timer firing here? 

    if (disposed) return; 
    if (t == null) return; 

    t.Dispose(); 
    disposed = true; 
    } 
} 

EDIT - Dans mon code actuel j'ai serrures sur la liste à la fois Ajouter et OnTimer. Je les ai laissés par hasard quand j'ai simplifié mon code pour le poster. Essentiellement, je veux accumuler des données, en les traitant par lots. Au fur et à mesure que j'accumule, si j'obtiens 10 articles OU que cela fait 1 seconde depuis que j'ai traité les données pour la dernière fois, je vais traiter ce que j'ai jusqu'à présent et effacer ma liste.

Puisque la minuterie est jetable, j'ai mis en place le modèle jetable sur ma classe. Ma question est la suivante: Ai-je besoin d'une protection supplémentaire dans l'une ou l'autre méthode Dispose pour éviter les effets secondaires du déclenchement de l'événement de minuterie?

Je pourrais facilement désactiver la minuterie dans l'une ou l'autre ou les deux méthodes d'élimination. Je comprends que cela ne me rendrait pas forcément sûr à 100% car la minuterie pouvait se déclencher entre le moment où Dispose est appelée et la minuterie désactivée. En laissant de côté cette question pour l'instant, serait-il considéré comme une meilleure pratique de garder la méthode Dispose (s), au mieux de mes capacités, contre la possibilité de l'exécution de la minuterie?

Maintenant que j'y pense, je devrais probablement aussi considérer ce que je devrais faire si la liste n'est pas vide dans Dispose. Dans le modèle d'utilisation de l'objet est devrait être vide d'ici là, mais je suppose que tout est possible. Disons qu'il y avait des éléments dans la liste quand Dispose est appelé, serait-il bon ou mauvais d'aller de l'avant et d'essayer de les traiter? Je suppose que je pourrais mettre dans ce vieux commentaire fidèle:

public void Dispose() 
{ 
    if (ints != null && ints.Count > 0) 
    { 
    //Should never get here. Famous last words! 
    } 
} 

S'il y a des articles dans la liste est secondaire. Je ne suis vraiment intéressé que par savoir quelle est la meilleure pratique pour traiter les Temporisations et Disposer potentiellement activés.

Si cela est important, ce code se trouve réellement dans une bibliothèque de classes Silverlight. Il n'interagit pas du tout avec l'interface utilisateur.

EDIT:

J'ai trouvé ce qui ressemble à une assez bonne solution here. Une réponse, par jsw, suggère de protéger l'événement OnTimer avec Monitor.TryEnter/Monitor.Exit, en plaçant effectivement le code OnTimer dans une section critique. J'ai publié ce qui semble être une solution encore meilleure, du moins pour ma situation, d'utiliser une minuterie en une seule fois en réglant le temps à l'intervalle désiré et en définissant la période sur Timeout.Infini.

Pour mon travail, je veux seulement que la minuterie se déclenche si au moins un élément a été ajouté à la liste. Donc, pour commencer, mon minuteur est désactivé. Lorsque vous entrez Ajouter, désactivez la minuterie pour qu'elle ne se déclenche pas lors de l'ajout. Lorsqu'un élément est ajouté, traitez la liste (si nécessaire). Avant de quitter Ajouter, s'il y a des éléments dans la liste (c.-à-d. Si la liste n'a pas été traitée), activez le temporisateur avec l'intervalle restant et la période définie sur Timeout.Infini. Avec un temporisateur à déclenchement unique, il n'est même pas nécessaire de désactiver le temporisateur dans l'événement OnTimer car le temporisateur ne se déclenchera pas de toute façon. Je n'ai pas non plus besoin d'activer le minuteur lorsque je quitte OnTimer car il n'y aura aucun élément dans la liste. J'attendrai qu'un autre élément soit ajouté à la liste avant d'activer de nouveau le chronomètre à un coup. Merci!

EDIT - Je pense que c'est la version finale. Il utilise une minuterie one-shot. La minuterie est activée lorsque la liste passe de zéro membre à un membre. Si nous atteignons le seuil de nombre dans Ajouter, les éléments sont traités et la minuterie est désactivée. L'accès à la liste des articles est surveillé par un verrou. Je suis sur la clôture pour savoir si une minuterie one-shot m'achète beaucoup plus d'une minuterie "normale" autre que celle qu'elle tirera seulement quand il ya des éléments dans la liste et seulement 1 seconde après l'ajout du premier élément. Si j'utilise une minuterie normale, alors cette séquence pourrait arriver: Ajouter 11 éléments en succession rapide. L'ajout du 11ème provoque le traitement et la suppression des éléments de la liste. Supposons qu'il reste 0,5 seconde sur la minuterie. Ajouter 1 article de plus. Le temps se déclenchera dans environ 0,5 seconde et le seul élément sera traité et retiré. Avec la minuterie à un coup, la minuterie est réactivée lorsque l'élément 1 est ajouté et ne se déclenche pas jusqu'à ce qu'un intervalle d'une seconde (plus ou moins) ait été éliminé. Est-ce que ça importe? Probablement pas. Quoi qu'il en soit, voici une version que je pense raisonnablement sûre et qui fait ce que je veux qu'elle fasse.

using System.Threading; 

public class MyClass : IDisposable 
{ 
    private List<int> ints = new List<int>(); 

    private Timer t; 

    public MyClass() 
    { 
    //Create timer in disabled state 

    t = new Timer(this.OnTimer, null, Timeout.Infinite, Timeout.Infinite); 
    } 

    private void DisableTimer() 
    { 
    if (t == null) return; 

    t.Change(Timeout.Infinite, Timeout.Infinite); 
    } 

    private void EnableTimer() 
    { 
    if (t == null) return; 

    //Fire event in 1 second but no events thereafter. 
    EnableTimer(1000, Timeout.Infinite); 
    } 

    private void DoSomethingWithTheInts() 
    { 
    foreach (int i in ints) 
    { 
     Whatever(i); 
    } 
    } 

    private void OnTimer(object state) 
    { 
    lock (ints) 
    { 
     if (disposed) return; 
     DoSomethingWithTheInts(); 
     ints.Clear(); 
    } 
    } 

    public void Add(int i) 
    { 
    lock(ints) 
    { 
     if (disposed) return; 

     ints.Add(i); 
     if (ints.Count > 10) 
     { 
     DoSomethingWithTheInts(); 
     ints.Clear(); 
     } 

     if (ints.Count == 0) 
     { 
     DisableTimer(); 
     } 
     else 
     if (ints.Count == 1) 
     { 
     EnableTimer(); 
     } 
    } 
    } 

    bool disposed = false; 

    public void Dispose() 
    { 
    if (disposed) return; 

    Dispose(true); 
    } 

    protected virtual void Dispose(bool disposing) 
    { 
    lock(ints) 
    { 
     DisableTimer(); 
     if (disposed) return; 
     if (t == null) return; 

     t.Dispose(); 
     disposed = true; 
    } 
    } 
} 

Answer 1

Je vois quelques problèmes sérieux avec la synchronisation dans ce code. Je pense que ce que vous essayez de résoudre est un problème classique de lecteur-rédacteur. Avec l'approche actuelle, il est très probable que vous allez rencontrer des problèmes, comme si quelqu'un essayait de modifier la liste lors du traitement?

Je recommande fortement d'utiliser des extensions parallèles pour .net ou (en utilisant .net 3.5 ou une version antérieure) en utilisant des classes telles que ReaderWriterlock ou même un simple mot clé Lock. Rappelez-vous également que System.Threading.Timer est un appel asynchrone, donc le OnTimer est appelé à partir du thread SEPARATE (à partir d'un .net ThreadPool) donc vous avez certainement besoin d'une synchronisation (comme éventuellement verrouiller la collection).

Je regarderais des collections concurrentes dans .NEt 4 ou utiliserais des primitives de synchronisation dans .net 3.5 ou plus tôt. NE désactivez pas le temporisateur dans la méthode ADD. Écrire un code correct dans OnTimer comme

lock(daObject) 
{ 
    if (list.Count > 10) 
     DoSTHWithList; 
} 

Ce code est la plus simple (mais pas optimale definetely) qui devrait fonctionner. Un code similaire doit également être ajouté à la méthode Add (verrouillage de la collection). Espérons que cela aide, sinon msg moi. luke

Answer 2

Vous ne devriez pas avoir à désactiver la minuterie pendant OnTimer être parce que vous avez un verrou autour de l'appel, d'où tous les fils attendent le premier à terminer ...