Потокобезопасный разрушаемый класс запуска событий в C # - PullRequest
2 голосов
/ 02 ноября 2010

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

Вопрос:

Реализуйте потокобезопасный класс, который запускает событие каждую секунду от построения.Должна быть функция для определения истекших секунд.Этот класс должен реализовывать IDisposable, и любые вызовы истекшей секунды после вызова dispose должны завершиться неудачей.

Мое решение:

namespace TimeCounter
{
public delegate void SecondsElapsedHandler(object o, EventArgs e);
/// <summary>
/// Summary description for SecondCounter
/// </summary>
public class SecondCounter : IDisposable
{
    private volatile int nSecondsElapsed;
    Timer myTimer;
    private readonly object EventLock = new object();
    private SecondsElapsedHandler secondsHandler;
    public SecondCounter()
    {
        nSecondsElapsed = 0;
        myTimer = new Timer();
        myTimer.Elapsed += new ElapsedEventHandler(OneSecondElapsed);
        myTimer.Interval = 1000;
        myTimer.AutoReset = false;
        myTimer.Start();
    }

    public void OneSecondElapsed(object source, ElapsedEventArgs e)
    {
        try
        {
            SecondsElapsedHandler handlerCopy;
            lock (EventLock)
            {
                handlerCopy = secondsHandler;
                nSecondsElapsed++;

            }
            if (secondsHandler != null)
            {
                secondsHandler(this, e);
            }
        }
        catch (Exception exp)
        {
            Console.WriteLine("Exception thrown from SecondCounter OneSecondElapsed " + exp.Message);
        }
        finally
        {
            if (myTimer != null)
            {
                myTimer.Enabled = true;
            }
        }
    }

    public event SecondsElapsedHandler AnotherSecondElapsed
    {
        add
        {
            lock (EventLock)
            {
                secondsHandler += value;
            }
        }
        remove
        {
            lock (EventLock)
            {
                secondsHandler -= value;
            }

        }
    }

    public int SecondsElapsed()
    {
        if (this.IsDisposed)
        {
            throw new ObjectDisposedException("SecondCounter");
        }
        return nSecondsElapsed;

    }

    private bool IsDisposed = false;
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
    private void Dispose(bool Disposing)
    {
        if (!IsDisposed)
        {
            if (Disposing)
            {

            }
            if (myTimer != null)
            {
                myTimer.Dispose();
            }

        }
        secondsHandler = null;
        IsDisposed = true;

    }
    ~SecondCounter()
    {
        Dispose(false);
    }
}
}

Ответы [ 2 ]

2 голосов
/ 03 ноября 2010

Есть несколько проблем:

  1. Возможно, вы были оштрафованы за проглатывание общего исключения, хотя это не имеет прямого отношения к проблемам многопоточности.

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

  3. Вы никогда не устанавливаете myTimer в null в Dispose.

  4. Вы получаете доступ к управляемому классу myTimer из финализатора (распоряжение = false), что является плохой идеей.

  5. Явная реализация события с блокировкой не требуется. Делегаты являются неизменными, и добавление / удаление события никогда не приведет к недопустимому состоянию делегата, хотя могут быть условия гонки, если делегаты добавляются / удаляются примерно в то же время, когда вызывается обратный вызов. Если вы используете стандартное объявление 'public event' без явно поддерживаемого частного делегата, синхронизация будет выполняться автоматически.

  6. (второстепенная точка) Если вы реализуете полный шаблон Dispose, обычно помечают метод Dispose (удаление в bool) как protected virtual, чтобы производные классы могли подключиться к механизму удаления. Еще лучше, отметьте свой класс sealed, и вы можете полностью исключить финализатор.

0 голосов
/ 03 ноября 2010

Ваш финализатор, вероятно, неисправен.Он правильно передает false в качестве параметра Disposing.Это должно сказать Dispose(bool), чтобы не пытаться утилизировать другие управляемые объекты.Но в этом методе вы вводите:

if (Disposing)
{

}
if (myTimer != null)
{
    myTimer.Dispose();
}

Таким образом, вы игнорируете значение Disposing.Это означает, что вы вызываете метод таймера Dispose из потока финализатора, когда этот объект, возможно, уже был завершен (если у него есть финализатор, что он, вероятно, делает).Финализаторы работают в непредсказуемом порядке.Обычно рекомендуется не выполнять вызовы для других объектов, управляемых GC, из финализатора.

Фактически, обычно рекомендуется не писать финализаторы вообще в эти дни.Вопрос не просил вас написать один!К сожалению, большинство уроков по IDisposable также говорят о финализаторах.Это разные предметы.

Вы также ловите Exception, базовый класс универсальных исключений.Это означает, что вы ловите такие вещи, как NullReferenceException.Обычно это не очень хорошая идея.Вы также входите в консоль, которая не имеет большого значения в графическом интерфейсе или серверном приложении.

Вы можете заменить:

myTimer.Elapsed += new ElapsedEventHandler(OneSecondElapsed);

на:

myTimer.Elapsed += OneSecondElapsed;

Ваше имя переменной не соответствует.См. рекомендации Microsoft .

...