Хабр Курсы для всех
РЕКЛАМА
Практикум, Хекслет, SkyPro, авторские курсы — собрали всех и попросили скидки. Осталось выбрать!
Хабр доступен 24/7 благодаря поддержке друзей
Комментарии 32
В задаче 2, извините, говнокод. Понятно, что после Free ссылка на obj не обнулится, но никто не гарантирует, что внутри Remove не будет попытки разыменования obj.
Даже если внутри Remove не обращаются к состоянию Obj, а используют только указатель в качестве ключа, то в многопоточном приложении это может привести к проблемам. После вызова Obj.Free в коллекцию из другого потока могут успеть положить новый объект по этому же адресу.
TDictionary не thread safe. Удаление без блокировок не корректно в принципе. Поэтому давайте для простоты положим, что код однопоточный.
А код в секции //DoSomething может влиять? Я уже больше 10 лет не брал в руки шашки Delphi, нужно по хорошему в дизассемблере смотреть что там происходит. Возможно оптимизации влияют на поведение.
По первой задаче, все может развалится если в параметр AExtraData засунуть ссылку на элемент из массива при условии что на него больше никто не ссылается. Например вызов AddDataToAll(DataArr[0]) уничтожит объект в DataArr[0] на первой итерации, а на второй мы можем развалиться (use after free).
По первой задаче, все может развалится если в параметр AExtraData засунуть ссылку на элемент из массива при условии что на него больше никто не ссылается. Например вызов AddDataToAll(DataArr[0]) уничтожит объект в DataArr[0] на первой итерации, а на второй мы можем развалиться (use after free).
Да, я думаю, что вы близки к истине, учитывая, что автор пишет, что данный код иногда падает. А «иногда» — очень хороший признак многопоточности.
obj сначала нужно удалить из коллекции, а потом делать obj.Free, не?
первый «пример» — не понятно чего автор ожидал от кода, при условии, что интерфейсы относятся к сущностям с автоматическим управлением памятью, т.е. как только ссылок на элемент не останется, он будет уничтожен
Представьте например, что Data1 и Data2 — хранят числа, а CreateMerged_IData создает новый IData с суммой этих чисел.
Вызывая procedure AddDataToAll(const AExtraData: IData); я ожидаю, что число, хранящееся в AExtraData добавится ко всем числам, хранящимся в DataArr (ну и в DataArr само собой будут новые интерфейсы, которые хранят новую сумму).
Вызывая procedure AddDataToAll(const AExtraData: IData); я ожидаю, что число, хранящееся в AExtraData добавится ко всем числам, хранящимся в DataArr (ну и в DataArr само собой будут новые интерфейсы, которые хранят новую сумму).
думаю, что const нужно прописать у параметров функции, чтоб подсчёт ссылок не путался под ногами при вызове, тем более по коду один и тот же элемент массива сначала передаётся в функцию и внутри неё заменяется.
А вообще стиль кодирования в этих примерах какой-то «опасный», «неряшливый». То интерфейсы в функцию без const, то объект освобождаем, а потом используем. Страшно…
А вообще стиль кодирования в этих примерах какой-то «опасный», «неряшливый». То интерфейсы в функцию без const, то объект освобождаем, а потом используем. Страшно…
AddData(DataArr[0], AExtraData, DataArr[0]); => AddData(DataArr[i], AExtraData, DataArr[i]);Миль пардон. Это уже моя ошибка когда набирал пример. Большое спасибо за замечание, поправил. :) Подводный камень там в другом.
Во втором — не верный порядок удаления элемента.Ну если допустить, что так (хеш берется от указателя), то объяснить почему можете?
Ну если допустить, что так (хеш берется от указателя), то объяснить почему можете?
habrahabr.ru/post/269359/#comment_8624505
«obj.Free; obj := nil» => FreeAndNil (obj)
была бы привычка у автора использовать FreeAndNil, не было бы «загадки» 2
была бы привычка у автора использовать FreeAndNil, не было бы «загадки» 2
во поводу 1:
лучше использовать функцию, вместо «процедура + out parameter»
а если пример не упрощён, то ошибка в том, что DataArr не инициализирован (нет SetLength(DataArr, ...)), а все эти интерфейсы — так, отвлекающий манёвр
лучше использовать функцию, вместо «процедура + out parameter»
function AddData(Data1, Data2: IData): IData;
begin
Result := CreateMerged_IData(Data1.Ptr, Data2.Ptr); //функция Create_IData создает новый экземпляр IData
end;
var DataArr: array of IData;
procedure AddDataToAll(const AExtraData: IData);
var i: Integer;
begin
if not Assigned(AExtraData) then
Exit;
for i := 0 to Length(DataArr) - 1 do
DataArr[i] := AddData(DataArr[i], AExtraData);
end;
а если пример не упрощён, то ошибка в том, что DataArr не инициализирован (нет SetLength(DataArr, ...)), а все эти интерфейсы — так, отвлекающий манёвр
Если в первом примере в AddDataToAll
строку
AddData(DataArr[i], AExtraData, DataArr[i]);
заменить на
var i: Integer; temp: IData;
temp := DataArr[i];
AddData(temp, AExtraData, DataArr[i]);
то всё работает.
Причина, видимо, в том, что при вызове процедуры AddData out параметр инициализируется nil.
а так как он ссылается на «живой» параметр, то, соответственно, в массиве по этому индексу окажется nil.
но такое поведение out параметров описано в справке Delphi.
Я лично на Дельфи не пишу, но те, кто использует Дельфи должны о таком поведении знать.
строку
AddData(DataArr[i], AExtraData, DataArr[i]);
заменить на
var i: Integer; temp: IData;
temp := DataArr[i];
AddData(temp, AExtraData, DataArr[i]);
то всё работает.
Причина, видимо, в том, что при вызове процедуры AddData out параметр инициализируется nil.
а так как он ссылается на «живой» параметр, то, соответственно, в массиве по этому индексу окажется nil.
но такое поведение out параметров описано в справке Delphi.
Я лично на Дельфи не пишу, но те, кто использует Дельфи должны о таком поведении знать.
кхм, я не спец по дельфям, но набросал у себя в блокноте следующее (пусть будет и здесь, чтобы потом сравнить с правильными ответами):
Задача 1:
не знаю, но подозреваю:
уж не будет ли выполнен OutData.Release() в функции AddData() до вызова функции CreateMerged_IData() ???
(что-то мне звучит бредово, но других мыслей нет, да и когда в дельфях не было досадных багов и/или недоработок? по-хорошему, надо в дизассемблере это предположение проверять)
Задача 2:
После выполнения obj.Free() переменная obj сохраняет значение (obj <> nil), и в старых дельфях код должен работать как часы…
Но как быть с современными версиями дельфей, которые умеют «автоматический подсчет ссылок» ???
( инструкция «obj := nil» по сути есть «пинок» нынешним дельфям для выполнения «obj.Release()», но память obj^ к этому моменту может быть уже невалидной! разве нет? )
P.S.:
отчаянно не хватает информации о версии Delphi, на которой код работает неправильно ))
Задача 1:
не знаю, но подозреваю:
уж не будет ли выполнен OutData.Release() в функции AddData() до вызова функции CreateMerged_IData() ???
(что-то мне звучит бредово, но других мыслей нет, да и когда в дельфях не было досадных багов и/или недоработок? по-хорошему, надо в дизассемблере это предположение проверять)
Задача 2:
После выполнения obj.Free() переменная obj сохраняет значение (obj <> nil), и в старых дельфях код должен работать как часы…
Но как быть с современными версиями дельфей, которые умеют «автоматический подсчет ссылок» ???
( инструкция «obj := nil» по сути есть «пинок» нынешним дельфям для выполнения «obj.Release()», но память obj^ к этому моменту может быть уже невалидной! разве нет? )
P.S.:
отчаянно не хватает информации о версии Delphi, на которой код работает неправильно ))
отчаянно не хватает информации о версии Delphi, на которой код работает неправильно ))
Это не важно. Это не баг, а подводный камень. Код работает правильно, но не очевидно на первый взгляд. Так что будет проявляться во всех версиях делфи.
ну, я о том, что старые версии дельфей не умели делать подсчет ссылок для объектов, (и ссылки подсчитывались только для интерфейсных переменных), а нынешние — умеют, хотя вроде есть директива, это поведение отключающая
ой, ну да, вторая задача по-любому только на современных версиях дельфей работать будет, там же генерик используется ))
что-то сразу и не обратил внимания на генерик )
что-то сразу и не обратил внимания на генерик )
2) С учётом, что TObject.Free ничего страшного не делает, остается вариант, что в FCollection.Remove(obj) приходит нулевой указатель, а из дельфийского папира вроде как выходит, что значение nil недопустимо
Так как у камрадов появился вопрос: «как в Remove может попасть nil, если перед этим стоит Obj.Free, оно же должно крешится на этом».
Это не так:
var Obj: TObject;
…
Obj := nil;
Obj.Free;
не вызовет никаких «крешей». Возможно, что именно эта строчка — NIL.Free рассматривается как неочевидное и невероятное.
Это не так:
var Obj: TObject;
…
Obj := nil;
Obj.Free;
не вызовет никаких «крешей». Возможно, что именно эта строчка — NIL.Free рассматривается как неочевидное и невероятное.
Опубликовал ответы.
спасибо!
очень интересные ответы. особенно по первой задаче )
я некоторых тонкостей не знал в плане передачи интерфейсных параметров, а про «out-» и вовсе не подозревал, как именно там устроено.
буду знать… и для надежности сохранил страничку на жесткий диск )
очень интересные ответы. особенно по первой задаче )
я некоторых тонкостей не знал в плане передачи интерфейсных параметров, а про «out-» и вовсе не подозревал, как именно там устроено.
буду знать… и для надежности сохранил страничку на жесткий диск )
Пожалуйста. Жаль только, что в целом статью отхабрили.
в смысле? это значит, что плюсов нету?
ну, сорри, я бы плюсанул, но у меня «недостаточно кармы для голосования» ((
ну, сорри, я бы плюсанул, но у меня «недостаточно кармы для голосования» ((
В текущий момент минусов больше чем плюсов. Я за плюсами не гоняюсь, просто когда статья набирает хоть что-то — видно что народу интересно, а сейчас я не вижу смысла писать еще подобные статьи про «подводные камни».
p.s. А вообще давайте заканчивать говорить про плюсы/минусы. Тут такое не в почете, да и сам я это не люблю. Просто сделал для себя выводы касательно этой тематики статей.
p.s. А вообще давайте заканчивать говорить про плюсы/минусы. Тут такое не в почете, да и сам я это не люблю. Просто сделал для себя выводы касательно этой тематики статей.
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
Что не так с этим кодом?