2010-11-02 2 views
0

У меня есть класс, который создает и использует System.Threading.Timer, что-то вроде этого:Таймер и IDisposable - Дополнительная защита в утилизации?

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 - В моей фактической кода у меня есть замки на список в обоих Добавляйте и OnTimer. Я случайно оставил их, когда я упростил свой код для публикации.

По существу, я хочу скопировать некоторые данные, обрабатывая их партиями. Поскольку я накапливаю, если я получаю 10 элементов ИЛИ прошло 1 секунду с тех пор, как я обработал последние данные, я обработаю то, что у меня есть, и очистить список.

Поскольку Timer is Disposable, я применил шаблон Disposable для своего класса. Мой вопрос заключается в следующем: нужна ли дополнительная защита в методе Dispose для предотвращения любых побочных эффектов при запуске события таймера?

Я мог бы легко отключить таймер в любом или обоих методах удаления. Я понимаю, что это не обязательно заставило бы меня на 100% безопаснее, поскольку таймер может срабатывать между временем вызова Dispose и отключением таймера. Оставив этот вопрос в стороне на данный момент, было бы лучше всего защищать метод (ы) Dispose, насколько это возможно, от возможности выполнения таймера?

Теперь, когда я думаю об этом, я должен, вероятно, также подумать над тем, что мне следует делать, если List не пуст в Dispose. В шаблоне использования объекта должен быть пустым к тому времени, но я думаю, что все возможно. Предположим, что в списке были оставлены элементы, когда вызывается Dispose, было бы хорошо или плохо идти вперед и пытаться их обработать? Я полагаю, что я мог бы поставить этот надежный старый комментарий:

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

Независимо от того, есть ли в списке предметы, являющиеся вторичными. Мне действительно интересно узнать, что лучше всего подходит для работы с потенциально включенными таймерами и утилитами.

Если это имеет значение, этот код фактически находится в библиотеке классов Silverlight. Он вообще не взаимодействует с пользовательским интерфейсом.

EDIT:

Я нашел то, что выглядит как довольно хорошее решение here. Один ответ на jsw предлагает защитить событие OnTimer с помощью Monitor.TryEnter/Monitor.Exit, эффективно помещая код OnTimer в критический раздел.

Michael Burr опубликовал то, что кажется еще лучшим решением, по крайней мере для моей ситуации, использования таймера с одним выстрелом, установив время ожидания на нужный интервал и установив период в Timeout.Infinite.

Для моей работы я хочу, чтобы таймер загорелся, если хотя бы один элемент был добавлен в список. Итак, для начала мой таймер отключен. После ввода «Добавить» отключите таймер, чтобы он не срабатывал во время добавления. Когда элемент добавлен, обработайте список (при необходимости). Перед тем, как оставить Add, если в списке есть какие-либо элементы (т. Е. Если список не был обработан), включите таймер с назначенным временем оставшимся интервалом и периодом, установленным на Timeout.Infinite.

С таймером с одним выстрелом нет необходимости отключать таймер в событии OnTimer, так как таймер не будет запускаться снова в любом случае. Мне также не нужно включать таймер, когда я оставляю OnTimer, поскольку в списке не будет никаких элементов. Я подожду, пока в список не будет добавлен другой элемент, прежде чем снова включить таймер с одним выстрелом. Спасибо!

EDIT - Я думаю, что это окончательная версия. Он использует таймер с одним выстрелом. Таймер включен, когда список переходит от нуля к одному элементу. Если мы нажмем порог счета в «Добавить», элементы будут обработаны, и таймер будет отключен. Доступ к списку предметов охраняется блокировкой. Я нахожусь на заботе о том, что таймер с одним выстрелом покупает меня намного больше, чем обычный «таймер», кроме того, что он срабатывает только при наличии элементов в списке и только через 1 секунду после добавления первого элемента. Если я использую обычный таймер, то эта последовательность может произойти: добавьте 11 пунктов в быстрой последовательности. Добавление 11-го приводит к тому, что элементы обрабатываются и удаляются из списка. Предположим, что на таймере осталось 0,5 секунды. Добавьте еще 1 элемент. Время срабатывает примерно через 0,5 секунды, и один элемент будет обработан и удален. С помощью таймера с одним выстрелом таймер снова включается, когда 1 элемент добавляется и не срабатывает до тех пор, пока не будет достигнут полный 1-секундный интервал (более или менее). Это имеет значение? Возможно нет. В любом случае, вот версия, которая, по моему мнению, достаточно безопасна и делает то, что я хочу, чтобы она делала.

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; 
    } 
    } 
} 
+2

Если вы не защищаете против известного состояния гонки, он снова вернется и укусит вас. –

ответ

1

Я вижу некоторые серьезные проблемы с синхронизацией в этом коде. Я думаю, что вы пытаетесь решить, это классическая проблема с читателем. При существующем подходе очень вероятно, что вы столкнетесь с некоторыми проблемами, например What What, если кто-то пытается изменить список при его обработке?

Я настоятельно рекомендую использовать параллельные расширения для .net или (при использовании .net 3.5 или более ранних версий) с использованием таких классов, как ReaderWriterlock или даже простого ключевого слова Lock. Также помните, что System.Threading.Timer является асинхронным вызовом, поэтому OnTimer вызывается из потока SEPARATE (из .net ThreadPool), поэтому вам определенно требуется некоторая синхронизация (например, блокировка коллекции).

Я бы посмотрел на параллельные коллекции в .NEt 4 или использовал некоторые примитивы синхронизации в .net 3.5 или ранее. НЕ отключайте таймер в методе ADD. Написать правильный код в OnTimer как

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

Этот код является самым простым (хотя определенно не является оптимальным), которые должны работать. Также аналогичный код следует добавить в метод Add (блокировка коллекции). Надеюсь, это поможет, если не msg me. Лука

+0

Спасибо. Я оставил блокировки, когда упростил свой фактический код для публикации. Любопытно, почему я не должен отключать таймер во время OnTimer? Интервал должен быть таким, чтобы таймер не срабатывал во время OnTimer в любом случае, но если интервал бывает коротким, а OnTimer занимает много времени, OnTimer может срабатывать, а затем где я буду? Я предполагаю, что другой подход состоит в том, чтобы иметь флаг в OnTimer, который позволил бы мне закоротить любые повторные вызовы OnTimer. – wageoghe

+0

Если второй (и последующий) по времени срабатывает, когда первый продолжает обрабатываться, вы должны написать OnTimer таким образом, чтобы он выполнил правильную Thing (например, ничего не делал) или, возможно, собрал еще 10 предметов и сделал sth к ним (например, первый скопировал десять предметов и разблокировал коллекцию и теперь обрабатывает их, если обработка одного элемента занимает много времени). Помните, что таймер находится в отдельной ветке, поэтому он импульсирует, проверяет состояние класса, что-то делает и умирает ... – luckyluke

1

Вы не должны отключить таймер во время OnTimer быть причиной у вас есть блокировка вокруг вызова, следовательно, все потоки ждут первого и до конца ...

Смежные вопросы