Comments 18
Где можно использовать приём "Скрытый построитель"? Если осознанно — то, разве что, для написания "чистого непонятного кода": кода, формально соответствующего критериям "чистого кода" от теоретиков
Ну вообще-то любой "теоретик" чистого кода скажет, что метод Dispose должен именно что диспозить и ничего более. Иначе SRP и LSP идут лесом. А следовательно, здесь имеем некий буллшит, а не попытку в "чистый код". А сам кейс - еще одно подтверждение того, что любые авторитеты (кодеры MS в данном случае) лажают.
МС всегда использовала схему "мутного" и "не документированного" кода. Со времён 3.11, когда всякие "обвесы" и "тайные практики программирования" прямо косяками ходили.
И за МС наблюдается практика, когда она подобные велограблевилопеды немного "модернизировала" от обновления к обновлению, что порой приводило к "забавным"/"эффектным" последствиям.
Просто бизнес, ничего личного (с).
Я тут исхожу из бритвы Хеллона - скорее всего просто так было удобно написать данный конкретный компонент данному конкретному разработчику. Ну, например, чтобы гарантировать отсутствие утечек памяти при добавлении объектов в кэш (добавить без диспоуза невозможно, если диспоуз и осуществляет добавление! - гениально!)
По поводу паттерна "скрытый строитель" - он нужен как и всё скрытое, когда мы не хотим мучить пользователя деталями своей реализации, а хотим сразу дать ему "кнопочку" "ехать". Потому втихую посоздавать/поуничтожать объекты в удобный для нас момент жизненного цикла (управляемого пользователем библиотеки) - это нормально. А нестандарт жизненного цикла мы пользователю и не обещали!
Ещё во времена Windows 95 было замечено, что если писать по документации от МС, то часто получается медленно и коряво, при том, что аналог от самих МС работает быстро и гладко.
Dispose сам по себе, при таком неочевидном кейсе не гарантирует ну никак отсутствие утечек памяти. Получается скорее в обратную сторону - в тот момент, когда ты ожидаешь что объект диспоузнулся (и в следующий раз по логике ты должен получить ObjectDisposedException!) объект добавляется куда-то "на долгую память" и становится ещё менее очевидно, как правильно его оттуда выковыривать не вызывая сборку мусора по всем кругам ада.
Почему этот явно антипаттерн не переписали более очевидным способом - ну очень хотелось бы знать
А еще становится непонятно как НЕ добавить энтри в случае ошибки.
НЕ добавить — это как раз просто: НЕ вызывать Dispose() в случае ошибки при конфигурировании. Для этого даже не надо делать специальную обработку исключений, можно просто писать код подряд, сначала — установку свойств, в конце — Dispose()
Но это противоречит общепринятым практикам использования Dispose — через using…
А, и ещё это приведёт к утечке памяти если включено то странное отслеживание.
Но это противоречит общепринятым практикам использования Dispose — через using…
О том и статья, собственно. Да, это нехорошо и контринтуитивно, но кто-то из разработчиков .NET Core когда-то решил сделать так — и это уже не исправишь, не нарушив совместимость. Только вот почему это ещё и не документировано? Разумного объяснения (ну, кроме очевидного — лень/нетбабла) для этого я не нахожу.
А, и ещё это приведёт к утечке памяти если включено то странное отслеживание.
Вряд ли: тот код всего лишь переносит установленные свойства с добавляемого в элемента на следующий, который нужно будет добавить, не более того. Причем, ни Key, ни Value в число переносимых свойств не входят. Так что на отслеживание памяти это само по себе влиять не должно.
Проблема не во влиянии на отслеживание памяти, а в тот, что все забытые ICacheEntry выстраиваются в односвязные списки начиная с ссылки в AsyncLocal-переменной.
Да, они будут жить столько, сколько будет жить соответствующий ExecutionContext. Но, насколько я понимаю, в ASP.NET Core это будет недолго: там каждый запрос связан со своим ExecutionContext (например, именно через него работает HttpContextAccessor — реализция по умолчанию для службы IHttpContextaccessor, служащей ныне в качестве средства доступа к текущему HttpContext заменой для HttpContext.Current времен ASP.NET Framework).
То есть, AsyncLocal-ссылки в норме будут жить только в течение времени обрабоки запроса.
Так?
То есть, если моя библиотека предназначена для ASP.NET Core даже версии 6.0, то я могу смело забывать ссылку на независимый CacheEntry — долго он не проживует, т.к. долго не проживет и ExecutionContext, который тоже имеет ссылку него.
Проблема в том, что существует такая штука как воркеры. И мидлвари Kestrel (не путать с мидлварями ASP.NET Core).
По этому поводу можно было бы, покопавшись в потрохах, поспорить и ещё, конечно.
Но, в общем и в целом, вы меня убедили: негоже оставлять ссылку на свой объект в другом объекте (ExecutionContext), время жизни которого я не контролирую. Надежнее будет делать Dispose() всегда, а в случае ошибки — вызывать Remove.
Пошел править код библиотеки.
Почти так, но вы пропустили этапы .net core до 3.0 , но не суть.
Контекст как вы описали жил еще во времена IIS когда , контекст был жестко привязан в flow цепочки обработки. Сейчас контекст живет как обычный объект в разрезе жизни потока и сборщика мусора.
Пример с замыканиями описал тут (ниже) , когда контекст живет после отдачи клиенту ответа и закрытия соединения:
UPD к предыдущему комментарию. По результатам обсуждения Dispose для ICacheEntry лучше все-таки вызывать всегда — ибо есть шанс оставить ссылку на этот независимый элемент кэша в потенциально долгоживущем объекте текущего контекста выполнения (ExecutionContext).
Что до ошибочного элемент кэша, то проще всего сделать установку свойства Value самой последней операцией перед Dispose: по исходному коду видно, что если Value==null то MemoryCache.SetEntry вызываться не будет, то и элемент в кэше тоже сохранен не будет. В крайнем случае, можно в перехватчике исключения удалить сбойный элемент через IMemoryCache.Remove: если элемента с таким ключом нет, то этот вызов будет просто проигнорирован.
Dispose сам по себе, при таком неочевидном кейсе не гарантирует ну никак отсутствие утечек памяти.Ага, Dispose тут выполняет совсем другую работу. В принципе, если объекты Key и Value не ссылаются на неуправляемые ресурсы (прямо или косвенно), то очистка тут вообще не требуется: сам MemoryCache неуправляемую память не использует. А если ссылаются, то вызов Dispose() для них надо писать самому: в процессе установки свойств, до добавленния — в обработчике исключения в try… catch, а после добавления в кэш — в eviction callback.
дебилы, что еще сказать
Ну не знаю ребят насчет "неправильного использования" , всетаки такие изменения были .net core 2.1 для улучшения многопоточности , как я помню они убрал какие то локи в ситуации: когда внутри asp.net на запрос создавался системный поток , а потом пользователь внутри создавал свой поток без блокировки, и системный отпускался, отдавался Response клиенту и вызывался Dispose на МемориЧеч , ПРИ ЭТОМ Меморич использовался и внутри системного потока , так и внутри пользовательского потока (замыкание на мемчеч).
Осторожно: ICacheEntry