Что плохого в этом решении для блокировки и управления заблокированными исключениями? - PullRequest
6 голосов
/ 12 февраля 2011

Моя цель - соглашение о поточно-ориентированной функциональности и обработке исключений в моем приложении.Я относительно новичок в концепции управления потоками / многопоточности.Я использую .NET 3.5

Я написал следующий вспомогательный метод, чтобы обернуть все мои заблокированные действия после прочтения этой статьи http://blogs.msdn.com/b/ericlippert/archive/2009/03/06/locks-and-exceptions-do-not-mix.aspx,, которая была связана в ответ на этот вопрос, Монитор против блокировки .

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

public static class Locking
{

    private static readonly Dictionary<object,bool> CorruptionStateDictionary = new Dictionary<object, bool>(); 
    private static readonly object CorruptionLock = new object();

    public static bool TryLockedAction(object lockObject, Action action, out Exception exception)
    {
        if (IsCorrupt(lockObject))
        {
            exception = new LockingException("Cannot execute locked action on a corrupt object.");
            return false;
        }
        exception = null;
        Monitor.Enter(lockObject);
        try
        {
            action.Invoke();
        }
        catch (Exception ex)
        {
            exception = ex;
        }
        finally
        {
            lock (CorruptionLock)   // I don't want to release the lockObject until its corruption-state is updated.
                                    // As long as the calling class locks the lockObject via TryLockedAction(), this should work
            {
                Monitor.Exit(lockObject);
                if (exception != null)
                {   
                    if (CorruptionStateDictionary.ContainsKey(lockObject))
                    {
                        CorruptionStateDictionary[lockObject] = true;
                    }
                    else
                    {
                        CorruptionStateDictionary.Add(lockObject, true);
                    }
                }
            }
        }
        return exception == null;
    }

    public static void Uncorrupt(object corruptLockObject)
    {
        if (IsCorrupt(corruptLockObject))
        {
            lock (CorruptionLock)
            {
                CorruptionStateDictionary[corruptLockObject] = false;
            }
        }
        else
        {
            if(!CorruptionStateDictionary.ContainsKey(corruptLockObject))
            {
                throw new LockingException("Uncorrupt() is not valid on object that have not been corrupted."); 
            }
            else
            {
                //  The object has previously been uncorrupted.
                //  My thought is to ignore the call.
            }
        }
    }

    public static bool IsCorrupt(object lockObject)
    {
        lock(CorruptionLock)
        {
            return CorruptionStateDictionary.ContainsKey(lockObject) && CorruptionStateDictionary[lockObject];
        }
    }


}

Я использую класс LockingException для простоты отладки.

    public class LockingException : Exception
    {
        public LockingException(string message) : base(message) { }
    }

Вот пример использования класса, чтобы показать, как я собираюсь использовать это.

public class ExampleUsage
{
    private readonly object ExampleLock = new object();

    public void ExecuteLockedMethod()
    {
        Exception exception;
        bool valid = Locking.TryLockedAction(ExampleLock, ExecuteMethod, out exception);
        if (!valid)
        {
            bool revalidated = EnsureValidState();
            if (revalidated)
            {
                Locking.Uncorrupt(ExampleLock);
            }
        }
    }

    private void ExecuteMethod()
    {
        //does something, maybe throws an exception

    }

    public bool EnsureValidState()
    {
        // code to make sure the state is valid
        // if there is an exception returns false,

        return true;
    }
}

Ответы [ 4 ]

7 голосов
/ 21 февраля 2011

Ваше решение, кажется, не добавляет ничего, кроме сложности из-за гонки в TryLockedAction:


        if (IsCorrupt(lockObject))
        {
            exception = new LockingException("Cannot execute locked action on a corrupt object.");
            return false;
        }
        exception = null;
        Monitor.Enter(lockObject);

LockObject может стать «поврежденным», пока мы все еще ждем на Monitor.Enter, так что нетзащита.

Я не уверен, какое поведение вы хотели бы достичь, но, вероятно, это поможет разделить блокировку и управление состоянием:


class StateManager
{
    public bool IsCorrupted
    {
        get;
        set;
    }

    public void Execute(Action body, Func fixState)
    {
        if (this.IsCorrupted)
        {
            // use some Exception-derived class here.
            throw new Exception("Cannot execute action on a corrupted object.");
        }

        try
        {
            body();
        }
        catch (Exception)
        {
            this.IsCorrupted = true;
            if (fixState())
            {
                this.IsCorrupted = false;
            }

            throw;
        }
    }
}

public class ExampleUsage
{
    private readonly object ExampleLock = new object();
    private readonly StateManager stateManager = new StateManager();

    public void ExecuteLockedMethod()
    {
        lock (ExampleLock)
        {
            stateManager.Execute(ExecuteMethod, EnsureValidState);
        }
    }

    private void ExecuteMethod()
    {
        //does something, maybe throws an exception

    }

    public bool EnsureValidState()
    {
        // code to make sure the state is valid
        // if there is an exception returns false,

        return true;
    }
}

Кроме того, насколько я понимаюСуть статьи в том, что государственное управление сложнее при наличии параллелизма.Однако это все еще только проблема правильности состояния вашего объекта, которая ортогональна блокировке, и, вероятно, вам нужно использовать совершенно другой подход для обеспечения правильности.Например, вместо изменения какого-либо сложного состояния с помощью заблокированной области кода, создайте новое и, если оно выполнено успешно, просто переключитесь на новое состояние в одном простом назначении ссылки:


public class ExampleUsage
{
    private ExampleUsageState state = new ExampleUsageState();

    public void ExecuteLockedMethod()
    {
        var newState = this.state.ExecuteMethod();
        this.state = newState;
    }
}

public class ExampleUsageState
{
    public ExampleUsageState ExecuteMethod()
    {
        //does something, maybe throws an exception
    }
}

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

1 голос
/ 12 февраля 2011

Хотя это выглядит надежно, у меня есть три проблемы:

1) Производительность Invoke () при каждом заблокированном действии может быть очень высокой. 2) Что если действие (метод) требует параметров? Потребуется более сложное решение. 3) Растет ли CorruptionStateDictionary бесконечно? Я думаю, что метод uncorrupt () должен проблема удалить объект, а не установить ложные данные.

0 голосов
/ 03 августа 2011

Подход, который я бы предложил, состоял бы в том, чтобы иметь объект менеджера состояний блокировки с полем "inDangerState". Приложение, которому требуется доступ к защищенному ресурсу, запускается с использованием объекта lock-manager-object для получения блокировки; менеджер получит блокировку от имени приложения и проверит флаг inDangerState. Если он установлен, диспетчер сгенерирует исключение и снимет блокировку, разматывая стек. В противном случае менеджер вернет ID, доступный приложению, которое снимет блокировку на Dispose, но также может манипулировать флагом состояния опасности. Прежде чем переводить заблокированный ресурс в плохое состояние, необходимо вызвать метод IDisposable, который установит inDangerState и вернет токен, который можно использовать для его повторной очистки после восстановления заблокированного ресурса в безопасном состоянии. Если IDisposable имеет значение Dispose'd до повторной очистки флага inDangerState, ресурс будет «застрял» в состоянии «опасности».

Обработчик исключения, который может восстановить заблокированный ресурс в безопасное состояние, должен использовать токен для очистки флага inDangerState перед возвратом или распространением исключения. Если обработчик исключений не может восстановить заблокированный ресурс в безопасное состояние, он должен распространить исключение, когда задан inDangerState.

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

0 голосов
/ 25 февраля 2011
  1. Переместите тест IsCorrupt и Monitor.Enter внутри Try
  2. Переместите обработку набора коррупции из finally и в блок Catch (это должно выполняться, только если было сгенерировано исключение)
  3. Не снимать первичную блокировку до тех пор, пока не будет установлен флаг искажения (оставьте его в блоке финала)
  4. Не ограничивайте выполнение вызывающим потоком;либо переделайте его, либо добавьте в словарь соответствия, заменив bool пользовательским исполнением, и верните его с проверкой IsCorrupt
  5. Для Uncorrupt просто удалите элемент
  6. Есть некоторые проблемы споследовательность блокировки (см. ниже)

Это должно охватывать все основания

  public static class Locking
{
    private static readonly Dictionary<object, Exception> CorruptionStateDictionary = new Dictionary<object, Exception>();
    private static readonly object CorruptionLock = new object();
    public static bool TryLockedAction(object lockObject, Action action, out Exception exception)
    {
        var lockTaken = false;
        exception = null;
        try
        {
            Monitor.Enter(lockObject, ref lockTaken);

            if (IsCorrupt(lockObject))
            {
                exception = new LockingException("Cannot execute locked action on a corrupt object.");
                return false;
            }

            action.Invoke();
        }
        catch (Exception ex)
        {
            var corruptionLockTaken = false;
            exception = ex;
            try
            {
                Monitor.Enter(CorruptionLock, ref corruptionLockTaken);

                if (CorruptionStateDictionary.ContainsKey(lockObject))
                {
                    CorruptionStateDictionary[lockObject] = ex;
                }
                else
                {
                    CorruptionStateDictionary.Add(lockObject, ex);
                }
            }

            finally
            {
                if (corruptionLockTaken)
                {
                    Monitor.Exit(CorruptionLock);
                }
            }
        }
        finally
        {
            if (lockTaken)
            {
                Monitor.Exit(lockObject);
            }
        }
        return exception == null;
    }
    public static void Uncorrupt(object corruptLockObject)
    {
        var lockTaken = false;
        try
        {
            Monitor.Enter(CorruptionLock, ref lockTaken);
            if (IsCorrupt(corruptLockObject))
            {
                { CorruptionStateDictionary.Remove(corruptLockObject); }
            }
        }
        finally
        {
            if (lockTaken)
            {
                Monitor.Exit(CorruptionLock);
            }
        }

    }
    public static bool IsCorrupt(object lockObject)
    {
        Exception ex = null;
        return IsCorrupt(lockObject, out ex);
    }
    public static bool IsCorrupt(object lockObject, out Exception ex)
    {
        var lockTaken = false;
        ex = null;
        try
        {
            Monitor.Enter(CorruptionLock, ref lockTaken);
            if (CorruptionStateDictionary.ContainsKey(lockObject))
            {
                ex = CorruptionStateDictionary[lockObject];
            }
            return CorruptionStateDictionary.ContainsKey(lockObject);
        }
        finally
        {
            if (lockTaken)
            {
                Monitor.Exit(CorruptionLock);
            }
        }           
    }
}
...