Является ли злоупотреблением использование IDisposable и «using» в качестве средств для получения «ограниченного поведения» для обеспечения безопасности исключений?

112

Что-то, что я часто использовал в 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?

Иоганн Герелл
источник
23
+1 только за имена классов и методов :-). Ну, честно говоря, и собственно вопрос.
Joey
2
@Johannes: Да, я понимаю, почему - большинство людей просто ласкают свою скрипку, но я просто должен возразить против этого. ;-) И, кстати, +1 за схожесть вашего имени.
Johann Gerell
Возможно, вы можете использовать область lock (this) {..} для достижения того же результата в этом примере?
oɔɯǝɹ
1
@ oɔɯǝɹ: вы используете lock (), чтобы предотвратить одновременный доступ к чему-либо из разных потоков. Ничего общего с описываемым мной сценарием.
Иоганн Герелл
1
IScopeable без финализатора в следующей версии C # :)
Джон Рейнор,

Ответы:

33

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

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

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

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

Андраш Золтан
источник
2
Я считаю, что это тоже художественное использование слова «использование». Я думаю, что сообщение Самуала Джека со ссылкой на комментарий Эрика Ганнерсона подтверждает эту реализацию «использования». Я вижу отличные замечания по этому поводу как от Андраса, так и от Эрика.
IAbstract
1
Некоторое время назад я говорил об этом с Эриком Ганнерсоном, и, по его словам, команда разработчиков C # намеревалась обозначить области действия с помощью. Фактически, он постулировал, что если бы они были спроектированы с использованием до оператора блокировки, могло бы даже не быть оператора блокировки. Это был бы блок использования с вызовом монитора или что-то в этом роде. ОБНОВЛЕНИЕ: только что понял, что следующий ответ от Эрика Липперта, который также входит в языковую команду. ;) Возможно, сама команда C # не полностью согласна с этим? Контекстом моего обсуждения с Гуннерсоном был класс TimedLock
Haacked
2
@Haacked - Забавно, не правда ли? ваш комментарий полностью подтверждает мою точку зрения; что вы поговорили с одним парнем из команды, и он был полностью за; затем, указывая на Эрика Липперта ниже, из той же команды не согласен. С моей точки зрения; C # предоставляет ключевое слово, которое использует интерфейс, а также дает красивый шаблон кода, который работает во многих сценариях. Если мы не должны злоупотреблять этим, то C # должен найти другой способ добиться этого. В любом случае, дизайнерам, вероятно, нужно собрать здесь своих уток подряд! А пока: using(Html.BeginForm())сэр? не против, если я так сделаю, сэр! :)
Андрас Золтан
71

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

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

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

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

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

Если бы вы принесли это на проверку кода в моем офисе, первый вопрос, который я бы задала, будет: «Действительно ли правильно блокировать блокировку, если возникает исключение?» Из вашей программы явно очевидно, что эта штука агрессивно повторно блокирует блокировку, что бы ни случилось. Это правильно? Возникло исключение. Программа находится в неизвестном состоянии. Мы не знаем, выбросили ли Foo, Fiddle или Bar, почему они выбросили или какие мутации они произвели в другом состоянии, которое не было очищено. Сможете ли вы убедить меня, что в этой ужасной ситуации всегда будет правильным сделать повторную блокировку?

Может быть, а может и нет. Я хочу сказать, что с кодом, который был изначально написан, рецензент знает, что нужно задать вопрос . С кодом, который использует "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 заблокирован, но до того, как будет введена область с защитой от попытки, связанная с использованием. Но с версией "с использованием" я предполагаю, что это "ну и что?" ситуация. Очень жаль, если это произойдет, но я предполагаю, что «использование» используется только для вежливости, а не для изменения жизненно важного состояния программы. Я предполагаю, что если какое-то ужасное исключение прерывания потока произойдет точно в неподходящее время, тогда, ну что ж, сборщик мусора в конечном итоге очистит этот ресурс, запустив финализатор.

Эрик Липперт
источник
18
Хотя я бы ответил на этот вопрос иначе, я думаю, что это хорошо аргументированный пост. Однако я не согласен с вашим утверждением, что usingэто вопрос вежливости, а не правильности. Я считаю, что утечки ресурсов - это проблемы с правильностью, хотя часто они настолько незначительны, что не являются высокоприоритетными ошибками. Таким образом, usingблоки уже оказывают семантическое влияние на состояние программы, и то, что они предотвращают, - это не просто «неудобство» для других процессов, но, возможно, нарушенная среда. Это не означает, что другой тип семантического воздействия, подразумеваемый вопросом, также уместен.
kvb
13
Утечки ресурсов - это проблема корректности, да. Но отказ от использования «использования» не приводит к утечке ресурса; в конечном итоге ресурс будет очищен при запуске финализатора. Очистка может быть не такой агрессивной и своевременной, как хотелось бы, но она обязательно произойдет.
Эрик Липперт,
7
Замечательный пост, вот мои два цента :) Я подозреваю, что многие "злоупотребления" связаны с usingтем, что это аспектно-ориентированный ткач для бедняков на C #. Что действительно нужно разработчику, так это способ гарантировать, что какой-то общий код будет выполняться в конце блока без необходимости дублировать этот код. C # сегодня не поддерживает АОП (за исключением MarshalByRefObject и PostSharp). Однако преобразование компилятором оператора using позволяет достичь простого АОП за счет изменения семантики детерминированного распределения ресурсов. Хотя это и не является хорошей практикой, иногда
приятно отказаться
11
Хотя я в целом согласен с вашей позицией, бывают случаи, когда выгода от перенацеливания usingслишком привлекательна, чтобы отказываться от нее . Лучший пример, который я могу придумать, - это отслеживать и сообщать о времени выполнения кода: using( TimingTrace.Start("MyMethod") ) { /* code */ } опять же, это АОП - Start()фиксирует время начала до начала блока, Dispose()фиксирует время окончания и регистрирует активность. Он не меняет состояние программы и не зависит от исключений. Он привлекателен еще и тем, что позволяет избежать использования сантехники, а частое использование в качестве шаблона смягчает запутанную семантику.
LBushkin
6
Забавно, я всегда думал об использовании как о средстве поддержки RAII. Мне кажется довольно интуитивно понятным рассматривать блокировку как тип ресурса.
Брайан
27

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

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

где .SafeExecute(Action fribbleAction)метод оборачивает блок try- catch- finally.

herzmeister
источник
В этом примере вы пропустили входное состояние. Кроме того, убедитесь, что вы завершили try / finally, но вы ничего не вызываете в finally, поэтому, если что-то в лямбде выдает, вы не знаете, в каком состоянии вы находитесь.
Иоганн Герелл
1
Ах! Хорошо, я думаю, вы имеете в виду, что SafeExecuteэто Fribbleметод, который вызывает Unlock()и Lock()в завернутом try / finally. Тогда мои извинения. Я сразу увидел SafeExecuteобщий метод расширения и поэтому упомянул об отсутствии состояния входа и выхода. Виноват.
Johann Gerell
1
Будьте очень осторожны с этим приложением. В случае захваченных местных жителей могут быть некоторые опасные тонкие непреднамеренные продления жизни!
Джейсон
4
ЮК. Зачем прятать try / catch / finally? Делает ваш код скрытым, чтобы его могли прочитать другие.
Франк Швитерман
2
@Yukky Frank: Это была не моя идея что-то «скрывать», это была просьба опрашивающего. :-P ... Тем не менее, просьба, наконец, вопрос "Не повторяйся". У вас может быть много методов, для которых требуется один и тот же шаблонный «фрейм» для чистого получения / выпуска чего-либо, и вы не хотите навязывать это вызывающему (подумайте об инкапсуляции). Также можно было бы более четко называть такие методы, как .SafeExecute (...), чтобы достаточно хорошо передавать то, что они делают.
herzmeister
25

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

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

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

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

Дуг,

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

Сэмюэл Джек
источник
4
Эта цитата должна сказать, что он имел в виду, когда сказал «именно этот сценарий», поскольку я сомневаюсь, что он имел в виду этот будущий вопрос.
Франк Швитерман
4
@Frank Schwieterman: Я закончил цитату. Судя по всему, люди из команды C # действительно думали, что эта usingфункция не должна ограничиваться утилизацией ресурсов.
paercebal
11

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

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

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

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

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

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

Ганс Пассан
источник
Я думаю, что принудительный случай был приведен выше "herzmeister der welten", даже если OP, похоже, не понравился пример.
Benjamin Podszun
Да, я интерпретировал его SafeExecuteкак общий метод расширения. Теперь я понимаю, что это, вероятно, имелось в виду как Fribbleметод, который вызывает Unlock()и Lock()в завернутом try / finally. Виноват.
Johann Gerell
Игнорирование исключения не означает, что оно не подлежит восстановлению. Это означает, что он будет обработан выше в стеке. Например, исключения можно использовать для корректного выхода из потока. В этом случае вы должны правильно освободить свои ресурсы, в том числе заблокированные блокировки.
paercebal
7

Реальным примером является 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 в Janitor?

Майкл Штум
источник
По поводу вашего «Мне не нравится эта строка» - я на самом деле вкратце подумал о том, чтобы сделать это таким образом, когда формулировал вопрос, но я не хотел загромождать семантику моего вопроса. Но я вроде как согласен с вами. Вид. :)
Johann Gerell
1
Проголосовали за предоставленный пример от самой Microsoft. Обратите внимание, что в случае, когда вы упоминаете, должно быть создано особое исключение: ObjectDisposedException.
Antitoon
4

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

Саймон Стил
источник
4

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

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

Бенджамин Подсзун
источник
1
«Дворник» взят из старой статьи на C ++ Андрея Александреску о защитных устройствах прицела, еще до того, как ScopeGuard добрался до Loki.
Johann Gerell
@Benjamin: Мне нравится класс TransactionScope, я о нем раньше не слышал. Может быть, потому что я нахожусь в стране .Net CF, где это не входит ... :-(
Johann Gerell
4

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

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

Цитата из спецификации языка C # :

8.13 Оператор using

[...]

Ресурс является классом или структуры , которая реализует System.IDisposable, который включает в себя один метод с именем УДАЛЕНИЕ без параметров. Код, использующий ресурс, может вызвать 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 # для активного объекта не предоставляется, написано, что C # использует шаблон IDispose для кода, версия C ++ которого содержится в деструкторе. И, глядя на версию C ++, мы видим деструктор, который ожидает завершения внутреннего потока перед завершением, как показано в этом другом отрывке из статьи:

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

Так что, что касается Херба, он блестящий .

paercebal
источник
3

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

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

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

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

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

stakx - больше не участвует
источник
1

Это не оскорбительно. Вы используете их для того, для чего они созданы. Но вам, возможно, придется рассматривать друг друга в соответствии с вашими потребностями. Например, если вы выбираете «артистизм», вы можете использовать «using», но если ваш фрагмент кода выполняется много раз, то по соображениям производительности вы можете использовать конструкции «try» .. «finally». Потому что «использование» обычно подразумевает создание объекта.

Ата
источник
1

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

Я бы переименовал FrobbleJanitor, вероятно, во что-то вроде FrobbleLockSession.

Франк Швитерман
источник
Насчет переименования - я использовал «Дворник» из-за основной причины, по которой разобрался с Lock / Unlock. Я думал, это рифмуется с другими вариантами имени. ;-) Если бы мне пришлось использовать этот метод в качестве шаблона для всей моей базы кода, я бы определенно включил что-то более общее описательное, как вы подразумевали в слове «Сессия». Если бы это были старые времена C ++, я бы, вероятно, использовал FrobbleUnlockScope.
Johann Gerell