Неправильно ли использовать IDisposable и «использовать» как средство для получения «ограниченного поведения» для обеспечения безопасности исключений? - PullRequest
105 голосов
/ 20 января 2010

Что-то, что я часто использовал в C ++, позволяло классу A обрабатывать состояние входа и выхода для другого класса B через конструктор и деструктор A, чтобы убедиться, что если что-то в этой области выдает исключение, тогда B будет иметь известное состояние при выходе из области. Это не чистый RAII, как аббревиатура, но тем не менее, это установленный образец.

В C # я часто хочу сделать

class FrobbleManager
{
    ...

    private void FiddleTheFrobble()
    {
        this.Frobble.Unlock();
        Foo();                  // Can throw
        this.Frobble.Fiddle();  // Can throw
        Bar();                  // Can throw
        this.Frobble.Lock();
    }
}

Что нужно сделать следующим образом

private void FiddleTheFrobble()
{
    this.Frobble.Unlock();

    try
    {            
        Foo();                  // Can throw
        this.Frobble.Fiddle();  // Can throw
        Bar();                  // Can throw
    }
    finally
    {
        this.Frobble.Lock();
    }
}

если я хочу гарантировать состояние Frobble при возврате FiddleTheFrobble. Код будет лучше с

private void FiddleTheFrobble()
{
    using (var janitor = new FrobbleJanitor(this.Frobble))
    {            
        Foo();                  // Can throw
        this.Frobble.Fiddle();  // Can throw
        Bar();                  // Can throw
    }
}

где FrobbleJanitor выглядит примерно как

class FrobbleJanitor : IDisposable
{
    private Frobble frobble;

    public FrobbleJanitor(Frobble frobble)
    {
        this.frobble = frobble;
        this.frobble.Unlock();
    }

    public void Dispose()
    {
        this.frobble.Lock();
    }
}

И вот как я хочу это сделать. Теперь реальность настигает, поскольку для того, что я хочу использовать , требуется , чтобы FrobbleJanitor использовалось с using. Я мог бы посчитать это проблемой проверки кода, но что-то беспокоит меня.

Вопрос: Считается ли вышеупомянутое злоупотреблением using и IDisposable?

Ответы [ 12 ]

69 голосов
/ 20 января 2010

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

Я считаю это злоупотреблением по трем причинам.

Во-первых, потому что я ожидаю, что «использование» используется для использования ресурса и распоряжается им, когда вы закончите с ним . Изменение состояния программы не означает с использованием ресурса , а его изменение обратно не означает удаление чего-либо. Следовательно, «использование» для изменения и восстановления состояния является злоупотреблением; код вводит в заблуждение случайного читателя.

Во-вторых, потому что я ожидаю, что «использование» будет использоваться из вежливости, а не по необходимости . Причина, по которой вы используете «использование» для удаления файла, когда вы закончили с ним, не в том, что необходимо , чтобы сделать это, а в том, что он вежлив - кто-то другой возможно, вы ждете, чтобы использовать этот файл, поэтому сказать «сделано сейчас» - это морально правильная вещь. Я ожидаю, что смогу реорганизовать «использование» так, чтобы использованный ресурс удерживался дольше, а потом утилизировался, и что единственное влияние этого - слегка затруднить другие процессы . Блок "using", который семантически влияет на состояние программы , является оскорбительным, потому что он скрывает важную, требуемую мутацию состояния программы в конструкции, которая выглядит так, как будто она предназначена для удобства и вежливости, а не необходимости.

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

Если бы вы принесли это на рассмотрение кода в моем офисе, первый вопрос, который я бы задал: "Действительно ли правильно заблокировать ошибку, если возникнет исключение?" Из вашей программы совершенно очевидно, что эта штука агрессивно блокирует ошибку независимо от того, что происходит. Это правильно? Исключение было сгенерировано. Программа находится в неизвестном состоянии. Мы не знаем, бросили ли Фу, Фиддл или Бар, почему они бросили, или какие мутации они выполнили в другом состоянии, которые не были очищены. Можете ли вы убедить меня, что в этой ужасной ситуации всегда - правильная вещь, которую нужно сделать, чтобы снова заблокировать?

Может быть, а может и нет. Я хочу сказать, что с кодом, как он был изначально написан, рецензент кода знает, как задать вопрос . С кодом, который использует «использование», я не знаю, чтобы задать вопрос; Я предполагаю, что блок «using» выделяет ресурс, использует его некоторое время и вежливо избавляется от него, когда это сделано, а не то, что закрывающая скобка блока «using» мутирует мой состояние программы в исключительных обстоятельствах, когда произвольно много условий согласованности состояния программы были нарушены.

Использование блока «using» для семантического эффекта приводит к фрагменту этой программы:

}

чрезвычайно значимый. Когда я смотрю на эту единственную близкую скобку, я не сразу думаю, что «эта скобка имеет побочные эффекты, которые имеют далеко идущие последствия для глобального состояния моей программы». Но когда вы злоупотребляете «использованием» подобным образом, вдруг это происходит.

Второе, что я хотел бы спросить, если бы увидел исходный код, это "что произойдет, если возникнет исключение после разблокировки, но до ввода попытки?" Если вы запускаете неоптимизированную сборку, компилятор мог вставить инструкцию no-op перед попыткой, и возможно возникновение исключения прерывания потока в no-op. Это редко, но это случается в реальной жизни, особенно на веб-серверах. В этом случае разблокировка происходит, но блокировка никогда не происходит, потому что перед попыткой возникло исключение. Вполне возможно, что этот код уязвим для этой проблемы, и на самом деле должен быть написан

bool needsLock = false;
try
{
    // must be carefully written so that needsLock is set
    // if and only if the unlock happened:

    this.Frobble.AtomicUnlock(ref needsLock);
    blah blah blah
}
finally
{
    if (needsLock) this.Frobble.Lock();
}

Опять, может быть, может и нет, но Я знаю, чтобы задать вопрос . С версией «using» она подвержена той же проблеме: исключение прерывания потока может быть вызвано после того, как Frobble заблокирован, но до того, как будет введена защищенная от попыток область, связанная с использованием. Но с версией «использую», я предполагаю, что это «ну и что?» ситуация. К сожалению, если это произойдет, но я предполагаю, что «использование» только для того, чтобы быть вежливым, а не изменять жизненно важное состояние программы. Я предполагаю, что если какое-то ужасное исключение прерывания потока произойдет точно в неподходящее время, то, ну, в общем, сборщик мусора очистит этот ресурс в конечном итоге, запустив финализатор.

32 голосов
/ 20 января 2010

Я так не думаю, обязательно. Технически IDisposable - это , предназначенный для использования с вещами, которые имеют неуправляемые ресурсы, но тогда директива using - это просто отличный способ реализации общего шаблона try .. finally { dispose }.

Пурист будет утверждать «да, это оскорбительно», и в пуристическом смысле это так; но большинство из нас пишут код не с точки зрения пуристов, а с полу-художественной точки зрения. Использование конструкции «использование» таким способом, на мой взгляд, действительно художественно.

Вы, вероятно, должны поместить другой интерфейс поверх IDisposable, чтобы отодвинуть его немного дальше, объясняя другим разработчикам, почему этот интерфейс подразумевает IDisposable.

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

26 голосов
/ 20 января 2010

Если вам просто нужен какой-то чистый код в области видимости, вы можете также использовать лямбда-выражения, * la

myFribble.SafeExecute(() =>
    {
        myFribble.DangerDanger();
        myFribble.LiveOnTheEdge();
    });

, где метод .SafeExecute(Action fribbleAction) охватывает блок try - catch - finally.

24 голосов
/ 20 января 2010

Эрик Ганнерсон, который был в команде разработчиков языка C #, дал этот ответ почти на тот же вопрос:

Дуг спрашивает:

re: оператор блокировки с тайм-аутом ...

Я проделал этот трюк раньше, чтобы разобраться с общими шаблонами в многочисленных методах. Обычно блокировка получения , но есть некоторые другие . Проблема в том, что это всегда похоже на взлом, так как объект на самом деле не одноразовый, так как " call-back-the-the-end-a-scope-able ".

Дуга,

Когда мы решили [sic] использовать оператор, мы решили назвать его «использованием», а не чем-то более конкретным для удаления объектов, чтобы его можно было использовать именно для этого сценария.

11 голосов
/ 20 января 2010

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

void UseMeUnlocked(Action callback) {
  Unlock();
  try {
    callback();
  }
  finally {
    Lock();
  }
}

Но это становится немного неловко без лямд. Тем не менее, я использовал IDisposable, как и вы.

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

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

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

6 голосов
/ 20 января 2010

Примером из реальной жизни является BeginForm ASP.net MVC. В основном вы можете написать:

Html.BeginForm(...);
Html.TextBox(...);
Html.EndForm();

или

using(Html.BeginForm(...)){
    Html.TextBox(...);
}

Html.EndForm вызывает Dispose, а Dispose просто выводит тег </form>. Хорошая вещь в этом заключается в том, что скобки {} создают видимую «область видимости», которая облегчает просмотр того, что находится внутри формы, а что нет.

Я бы не стал злоупотреблять этим, но по сути IDisposable - это просто способ сказать: «Вы ДОЛЖНЫ вызывать эту функцию, когда закончите с ней». MvcForm использует его, чтобы убедиться, что форма закрыта, Stream использует его, чтобы убедиться, что поток закрыт, вы можете использовать его, чтобы убедиться, что объект разблокирован.

Лично я бы использовал его только в том случае, если выполняются следующие два правила, но они установлены мной произвольно:

  • Dispose должен быть функцией, которая всегда должна выполняться, поэтому не должно быть никаких условий, кроме Null-Checks
  • После Dispose () объект не должен использоваться повторно. Если бы я хотел использовать объект многократного использования, я бы предпочел использовать методы открытия / закрытия, а не распоряжаться ими. Поэтому я выкидываю исключение InvalidOperationException при попытке использовать удаленный объект.

В конце концов, все зависит от ожиданий. Если объект реализует IDisposable, я предполагаю, что он должен выполнить некоторую очистку, поэтому я вызываю его. Я думаю, что это обычно лучше, чем если бы у вас была функция выключения.

Как говорится, мне не нравится эта строка:

this.Frobble.Fiddle();

Поскольку FrobbleJanitor теперь "владеет" Frobble, мне интересно, не лучше ли вместо этого назвать Fiddle на его Frobble в Дворнике?

4 голосов
/ 27 августа 2010

Примечание: моя точка зрения, вероятно, смещена из моего фона C ++, поэтому значение моего ответа должно оцениваться с учетом этого возможного смещения ...

Что говорит спецификация языка C #?

Цитирование Спецификация языка C # :

8.13 Оператор использования

[...]

A resource - это класс или структура, реализующие System.IDisposable, который включает в себя единственный метод без параметров с именем Dispose. Код, использующий ресурс, может вызвать Dispose, чтобы указать, что ресурс больше не нужен. Если Dispose не вызывается, автоматическое удаление в конечном итоге происходит в результате сборки мусора.

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

Так что я думаю, это нормально, потому что Lock - это ресурс.

Возможно, ключевое слово using было выбрано неправильно. Возможно, его следовало назвать scoped.

Тогда мы можем рассматривать почти все как ресурс. Дескриптор файла. Сетевое подключение ... А нить?

Нить ???

Использовать (или злоупотреблять) ключевое слово using?

Было бы блестящим , чтобы (ab) использовать ключевое слово using, чтобы убедиться, что работа потока завершена до выхода из области действия?

Херб Саттер, кажется, думает, что это блестящий , поскольку он предлагает интересное использование шаблона IDispose для ожидания завершения работы потока:

http://www.drdobbs.com/go-parallel/article/showArticle.jhtml?articleID=225700095

Вот код, скопированный из статьи:

// C# example
using( Active a = new Active() ) {    // creates private thread
       …
       a.SomeWork();                  // enqueues work
       …
       a.MoreWork();                   // enqueues work
       …
} // waits for work to complete and joins with private thread

Хотя код C # для объекта Active не предоставлен, написано, что C # использует шаблон IDispose для кода, версия C ++ которого содержится в деструкторе. И, глядя на версию C ++, мы видим деструктор, который ждет завершения внутреннего потока перед выходом, как показано в этом другом фрагменте статьи:

~Active() {
    // etc.
    thd->join();
 }

Итак, что касается Травы, она блестящая .

4 голосов
/ 20 января 2010

Подсказка: я согласен с большинством здесь, что это хрупко, но полезно. Я хотел бы указать вам на класс System.Transaction.TransactionScope , который делает то, что вы хотите.

Обычно мне нравится синтаксис, он удаляет много беспорядка из настоящего мяса. Пожалуйста, подумайте над тем, чтобы дать вспомогательному классу хорошее имя, хотя ... может быть ... Область действия, как в примере выше. Имя должно указывать, что оно инкапсулирует кусок кода. * Scope, * Блок или что-то подобное должно сделать это.

4 голосов
/ 20 января 2010

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

3 голосов
/ 20 января 2010

Я считаю, что ответ на ваш вопрос - нет, это не будет злоупотреблением IDisposable.

Насколько я понимаю, интерфейс IDisposable состоит в том, что вы не должны работать с объектом после его удаления (за исключением того, что вы можете вызывать его метод Dispose столько раз, сколько захотите).

Поскольку вы создаете новый FrobbleJanitor явно каждый раз, когда вы получаете оператор using, вы никогда не будете использовать один и тот же объект FrobbeJanitor дважды. А поскольку его целью является управление другим объектом, Dispose кажется подходящим для задачи освобождения этого («управляемого») ресурса.

(Кстати, стандартный пример кода, демонстрирующий правильную реализацию Dispose, почти всегда предполагает освобождение управляемых ресурсов, а не только неуправляемых ресурсов, таких как дескрипторы файловой системы.)

Единственное, что лично меня беспокоит, это то, что менее понятно, что происходит с using (var janitor = new FrobbleJanitor()), чем с более явным блоком try..finally, где операции Lock & Unlock видны непосредственно. Но какой подход выбран, вероятно, сводится к вопросу личных предпочтений.

...