Comments 66
Скажите, а зачем вы запрещаете double dispose?
Прежде всего тем что если внешняя система пытается разрушить разрушенное — это значит что она плохо спроектирована.
Эээ?
using(Stream s = GetRequestStream())
{
using(XmlReader r = XmlReader.Create(s))
{
//...
}
}Собственно, написано же: "DO allow the Dispose(bool) method to be called more than once. [...] Users expect that a call to Dispose will not raise an exception."
Выглядит красиво, удобно, но не считаю это хорошей архитектурой.
Это типовое решение. И этот же стрим будет еще потом несколькими уровнями выше тоже закрыт, просто потому, что при закрытии реквеста его надо закрыть.
При этом внешний код, зная что объект разрушен ставит using, создавая дополнительный try… finally, который дополнительно замедляет приложение.
Есть конкретный бенчмарк, который показывает, насколько это медленнее?
Понимаете, если вы нарушите это соглашение (allow for multiple disposal), вы нарушите контракт, который предлагают разработчики фреймворка, тем самым создав бесконечное количество WTF. Не надо так.
Это типовое решение.
Это не типовое решение, а решение принятое разработчиками платформы. Возможно это был выбор меньшего зла. Но это не означает, что мы теперь везде должны делать тоже самое.
Впрочем, вас никто не заставляет повторно использовать Dispose в своём коде. Речь лишь о том, чтобы позволить так делать другим, когда вы проектируете IDisposable класс. И пусть это позволит кому-то писать менее оптимальный код. Ведь не заставит, а лишь позволит.
Если это был выбор меньшего зла, то зачем же нам теперь выбирать большее зло?
Чтобы не было недопонимания поясню. Я вижу два вопроса: 1) должен ли посторонний код диспозить объект, который он не создавал. 2) нужно ли запрещать повторный dispose.
И мой комментарий относился именно к первому вопросу, поскольку веткой выше обсуждался XmlReader. Я имел ввиду, что наименьшее зло — это то, что reader|writer диспозят stream. Однако, в других ситуациях такое решение может быть не оправдано. Хотя лично я и в части стримов, такое решение не оправдываю.
По поводу исключений в Dispose мне нечего сказать. Да, упадём в рантайме при повторном вызове. Но, не исключено, что повторный вызов говорит, о неверной работе программы. Исключение гораздо быстрее обнаруживается и отлаживается, чем скрытые баги. Думаю, это хотел донести автор.
должен ли посторонний код диспозить объект, который он не создавал.
Это извечная проблема. Нет четкого разделения между передачей во временное пользование, и передачей во владение. В первом случае диспозить нельзя, во втором — нужно. Тут не помешала бы move семантика. Что-то вроде плюсового unique_ptr.
Это как раз типовое решение в том смысле, что в .net-экосистеме (в BCL, в скачиваемых пакетах, в разработках) так уже принято. Вы можете так не делать — в том смысле, что вы можете никогда не захватывать контроль над переданным вам ресурсом — но вы не можете ожидать, что никто другой не будет так делать.
Как принято?
Закрывать переданный тебе ресурс, если известно, что ты потребил его полностью. Early release, вот это все.
Равно как и все равно закрывать ресурс, даже если ты его кому-то отдал, в момент, когда он теряет актуальность (например, поток запроса при окончании обработки запроса).
Так делают лишь упомянутый XmlReder и другие reader-ы/writer-ы.
Это уже достаточно много, чтобы понимать, что поток может быть закрыт кем угодно в любой момент.
Это типовое решение. И этот же стрим будет еще потом несколькими уровнями выше тоже закрыт, просто потому, что при закрытии реквеста его надо закрыть.
Скорее я бы назвал это типовой ошибкой. В данном случае все получилось замечательно, но добавляем одну маленькую деталь — и все валится :-)
public Message Foo()
{
using(Stream s = GetSomeStream())
{
using(XmlReader r = XmlReader.Create(s))
{
return Message.CreateMessage(r, int.MaxValue, MessageVersion.Soap12);
}
}
}
Например, ThreadAbortException, который не перехватывается ни catch ни finally.
Перехватывается
ThreadAbortException is a special exception that can be caught, but it will automatically be raised again at the end of the catch block. When this exception is raised, the runtime executes all the finally blocks before ending the thread.
Хороший пример: а вы знали чтоIEnumerable<T>тянет за собойIDisposable?
Не тянет (.net 4). Другое дело, что класс, реализующий IEnumerable, может одновременно реализовывать IDisposable, и foreach в C# это учтет.
Необходимое уточнение: IEnumerator<T> действительно требует IDisposable (в отличие, кстати, от IEnumerator).
Кажется, вы забыли упомянуть о еще одном важном достоинстве SafeHandle. Дело в том, что какой-нибудь ThreadAbortException может прилететь после вызова CreateFile — но еще до установки значения поля _handle. Без использования SafeHandle это приведет к редкой но очень неприятной утечке ресурса.
Оттуда удобно качать pdf/mobi/ebub, + автоматом формируются содержание и навигация для онлайн-версии. см. пример
Он должен работать как часы в любом положении, повторный вызов — частый кейс.
Называете статью полным описанием, но я так и не увидел хорошей рекомендации, как НАДО делать. А между тем, есть вполне себе отличный вариант на SO — ru.stackoverflow.com/a/486697/196257
Но согласен, стоит выделить итоги отдельно.
Где вы нашли рекомендацию кидать исключения?
А что делает ваш CheckDispose который вы вызываете внутрях Dispose?
Отдали вы в сторонюю либу свою реализацию IDisposable, а она падать начала на вашем исключении. Будете либу патчить? Удачи, время потрачено зря.
ПС: «Два уровня Disposable Design Principle» неудобный паттерн, т.к. смешивать управляемые и неуправляемые — неудобно и смешивает обычно уровни абстракций. Да, он популярный, нет, его не надо вставлять в статью, ибо он выглядит рекомендацией.
Я отстаиваю и буду до упора продолжать отстаивать позицию исключений при повторном вызове Dispose(). Эта "возможность" введена чтобы без лишнего геморроя разрушить граф объектов, например, начав с любого узла. Т.е. алгоритм криво спроектирован и позволяет себе повторные вызовы на одинаковых объектах. По моему скромному мнению это — защита от криворукости программистов с полным пониманием того что IDisposable, который они сделали для некоторых ситуаций требует сноровки чтобы все разрулить без повторных вызовов.
Отдали вы в сторонюю либу свою реализацию IDisposable, а она падать начала на вашем исключении. Будете либу патчить? Удачи, время потрачено зря.
Значит я криворук и отдал баг на продакшн, а QA фигово оттестировали.
Назовите мне, пожалуйста, реальный кейс где без повторного вызова — ну никак? :)
«Два уровня Disposable Design Principle» неудобный паттерн, т.к. смешивать управляемые и неуправляемые — неудобно и смешивает обычно уровни абстракций
Паттерн ничего не смешивает, а наоборот — разделяет ответственность. Level 0 = это, например, SafeHandle. Он заведует только IntPtr и все. Никаких управляемых ресурсов. Либо какой-то свой класс, унаследованный от CriticalFinalizerObject и инкапсулирующий, например IntPtr и другой SafeHandle. Тогда делается икремент счетчика ссылок чтобы порядок финализации задать. Level 1 = разрушение только управляемых ресурсов. Полное разделение ответственности
Реальный кейс я вам выше показал.
Что должен делать в такой ситуации разработчик? Я предпочитаю закрывать стрим, потому как он мной создан, мной и закрыт. А потом оказывается, что либа считает принятый в конструктор стрим — своим, закрывает сама. Только с вашим вариантом об этом узнать придётся в рантайме, поймав исключение, а с коробочным вариантом — не придётся даже заморачиваться. Будет просто работать. Как собственно и должен делать хороший код.
Однажды такое приложение завалится с OutOfMemoryExceptionНо стоит заметить, что вовсе не обязательно приложение падает с такого исключения, см. пример на скорую
Мне кажется, что тут в принципе стоит заметить, как себя ведёт приложение при различных исключениях:
- StackOverflowException — приложение падает в нуль и без кетчей. Чтобы избежать такого — можно чекать стек через RuntimeHelpers.EnsureSufficientExecutionStack
- ThreadAbortException — ловится в catch, но пробрасывается выше
- OutOfMemoryException — ловится в кетч, если по факту памяти уже достаточно, то приложение продолжает работу
- ExecutionEngineException — что-то не так в самом CLR пошло
Подробнее см. Exceptional Exceptions in .NET
Спасибо автору за статью и всем за коменты, они не менее интересны.
Если делать API для узкого круга лиц и/или для проекта в котором критична скорость освобождения ресурсов, то вариант автора намного более предпочтителен.
Если же делать API «для всех», то однозначно только вариант допускающий множественные Dispose, т.к. это уже давно устоявшаяся практика для «всех» и в 1-ую все должно быть надежно и интуитивно понятно.
Но с другой стороны C# ведь в принципе не предназначен для систем реального времени и возможно концепция автора просто не имеет смысла, а в случаях когда критична скорость освобождения ресурсов следует изначально выбирать C++ и т.п. и полностью контролировать ситуацию. Буду рад объективной критике.
Если делать API для узкого круга лиц и/или для проекта в котором критична скорость освобождения ресурсов, то вариант автора намного более предпочтителен.
Почему?
Именно что почему "вариант автора оказывается более предпочтителен, если критична скорость освобождения ресурсов". Какая именно особенность решения в посте повышает скорость освобождения?
Для начала, что такое "скорость освобождения ресурсов"? Время, затраченное на вызов Dispose? Или время от создания объекта до момента, когда его ресурсы освобождены (успешно вызван Dispose)?
Для меня важно лишь определиться имеет ли смысл использовать данный паттерн.
В общих случаях, я думаю всем очевидно, что нет. Т.к. страдает простота, красота, читабельность и т.д. кода, увеличивается объем инфраструктурного кода, может возникнуть куча проблем с проектированием и рефакторингом из ничего и т.д. И в итоге можно получить проектирование ради проектирования…
Как я писал выше возможно это может иметь смысл в каких-то частных случаях, как вариант оптимизации, но лично я не вижу каких-то явных плюсов в том, что ВОЗМОЖНО сократится количество вызовов Dispose такой ценой.
Я просто не могу представить задачу или ситуацию когда использование этого паттерна в контексте .NET будет оправдано. Автор в статье упоминал про граф объектов… В частном случае, если классы этих объектов описывают относительно примитивные сущности, целесообразным будет использование структур, а не классов со всеми вытекающими плюсами типов значений над ссылочными и это даст ощутимую оптимизацию быстродействия и использования памяти, если выделять память сразу для больших групп таких объектов.
Возможно не совсем верная аналогия, но это можно сравнить с авто с АКПП и МКПП. И использование этого паттерна это попытка ехать на АКПП как на МКПП. Но если критичны возможности МКПП, не лучше ли изначально выбрать МКПП.
Ведь основные преимущества C# это скорость разработки и высокая сложность отстрелить себе ноги, а в данном случае мы получается пытаемся идти против этих основных концепций.
Опять же, буду рад объективной критике и примеру того когда этот паттерн даст ощутимые плюсы от своего использования.
Насчет скорости освобождения ресурсов, я пожалуй не корректно сформулировал. Думаю можно считать этим суммарное время вызовов всех Dispose или количество вызовов Dispose.
Так оно не зависит от того, что внутри Dispose.
Для меня важно лишь определиться имеет ли смысл использовать данный паттерн.
Какой именно?
Так оно не зависит от того, что внутри Dispose.
А я разве утверждал обратное? В том то собственно и вопрос, что мы по сути пытаемся оптимизировать количество вызовов методов которые по сути ничего не делают и сразу вызывают return.
Какой именно?
Вариант автора статьи. Я ведь очень подробно изложил свои выводы.
А я разве утверждал обратное?
Вы утверждали, что "Если делать API для узкого круга лиц и/или для проекта в котором критична скорость освобождения ресурсов, то вариант автора намного более предпочтителен.". Хотя в реальности вариант автора статьи не окажет влияния на скорость освобождения ресурсов.
Вариант автора статьи.
В том-то и дело, что вариант автора статьи — это не паттерн.
Проще говоря, вариант автора статьи предпочтителен тогда и только тогда, когда кто-то зачем-то хочет заэнфорсить правило "строго один Dispose", причем из-за архитектурных соображений (потому что практической разницы нет).
В том-то и дело, что вариант автора статьи — это не паттерн.
Паттерн или не паттерн, или реализация паттерна это вопрос скорее философский и/или риторический.
Проще говоря, вариант автора статьи предпочтителен тогда и только тогда, когда кто-то зачем-то хочет заэнфорсить правило «строго один Dispose», причем из-за архитектурных соображений (потому что практической разницы нет).
Полностью согласен. Но для чего и зачем это в принципе может быть нужно?
Введение общеизвестного способа узнать, что конкретный тип требует разрушения его экземпляров в конце использования
Я думаю в этом кроется ваше желание кидать эксепшн при повторном вызове Dispose. У меня другое понимание: IDisposable — способ узнать, что конкретный тип требует освобождения захваченных ресурсов. Разрушение — это только одно из следствий вызова метода (на самое главное). Много где по тексту я бы заменил разрушение на освобождение.
Очень хорошая фундаментальная статья.
Есть пара-тройка замечаний:
- Поведение при вызове уже освобожденного (разрушенного) объекта логично считать неопределенным.
Это полный аналог проверок на null аргументов и результатов методов. Они полезны при отладке, но в релизе могут быть удалены по соображениям производительности: все равно к удаленному объекту без ошибок в коде никто не обращается. Добавление таких проверок вручную нецелесообразно (они чисто шаблонные), тут подходящие условия для применения АОП, например, специального плагина для Fody. - Поведение при двойном Dispose также логично считать непределенным из тех же соображений. Только тут есть два нюанса: передача IDisposable тому, кто не является владельцем (включая наследование интерфейсов от IDisposable) — плохой дизайн и этот плохой дизайн, к сожалению, широко распространен. В результате на практике повторный вызов Dispose лучше всего игнорировать (писать в лог в отладочной версии).
- Жесткий порядок освобождения и различие level-1 и level-0 для их владельца скорее вредно чем бесполезно. При отсутсвии ошибок дизайна (у каждого IDisposable ровно один владелец) все и так сработает хорошо, при наличии — все и так будет плохо.
- Критика паттерна на мой взгляд излишне суровая — возможно, лучше сосредоточиться на антипаттернах, связанных с неверным пониманием низкоуровневой природы IDisposable. Классический пример — требование передачи стримов и ридеров, реализующих IDisposable. Это сразу создает двух владельцев со всеми вытекающими.
- Главу про lifetime логично объединить с этой или поставить в книге сразу за ней.
Еще раз спасибо за глубину и точность сведений в статье.
Возможно, для реализации level-1 пригодится следующий паттерн:
https://habrahabr.ru/post/270929/
[DotNetBook] Реализация IDisposable: правильное использование