Pull to refresh

Comments 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).
код в секции //DoSomething не влияет на поведение
Да, я думаю, что вы близки к истине, учитывая, что автор пишет, что данный код иногда падает. А «иногда» — очень хороший признак многопоточности.
obj сначала нужно удалить из коллекции, а потом делать obj.Free, не?
первый «пример» — не понятно чего автор ожидал от кода, при условии, что интерфейсы относятся к сущностям с автоматическим управлением памятью, т.е. как только ссылок на элемент не останется, он будет уничтожен
Представьте например, что Data1 и Data2 — хранят числа, а CreateMerged_IData создает новый IData с суммой этих чисел.
Вызывая procedure AddDataToAll(const AExtraData: IData); я ожидаю, что число, хранящееся в AExtraData добавится ко всем числам, хранящимся в DataArr (ну и в DataArr само собой будут новые интерфейсы, которые хранят новую сумму).
думаю, что const нужно прописать у параметров функции, чтоб подсчёт ссылок не путался под ногами при вызове, тем более по коду один и тот же элемент массива сначала передаётся в функцию и внутри неё заменяется.

А вообще стиль кодирования в этих примерах какой-то «опасный», «неряшливый». То интерфейсы в функцию без const, то объект освобождаем, а потом используем. Страшно…
В первом — не используется переменная цикла:

AddData(DataArr[0], AExtraData, DataArr[0]); => AddData(DataArr[i], AExtraData, DataArr[i]);

Во втором — не верный порядок удаления элемента.

obj.Free;
FCollection.Remove(obj);
obj := nil;

=>

FCollection.Remove(obj);
obj.Free;
obj := nil;

Вроде все. Задачки для пятиклассника, если честно.
AddData(DataArr[0], AExtraData, DataArr[0]); => AddData(DataArr[i], AExtraData, DataArr[i]);
Миль пардон. Это уже моя ошибка когда набирал пример. Большое спасибо за замечание, поправил. :) Подводный камень там в другом.

Во втором — не верный порядок удаления элемента.
Ну если допустить, что так (хеш берется от указателя), то объяснить почему можете?
>Подводный камень там в другом.

В потере указателей на объекты? Создаем новый объект и подменяем ими старый. Сборщика мусора нет. Других проблем вроде не вижу (((

>Ну если допустить, что так (хеш берется от указателя), то объяснить почему можете?

Честно — нет, я уже подзабыл delphi капитальненько… Но, даже по простой логике — сначала удаляем из коллекции, потом из памяти). Как уже сказал Zapped, использовали-бы FreeAndNil — не было-бы подобного вопроса)
«obj.Free; obj := nil» => FreeAndNil (obj)
была бы привычка у автора использовать FreeAndNil, не было бы «загадки» 2
во поводу 1:
лучше использовать функцию, вместо «процедура + 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.
Я лично на Дельфи не пишу, но те, кто использует Дельфи должны о таком поведении знать.
Да, кстати, где-то когда-то мельком читал, что порядок вычисления выражений параметров компилятор может с оптимизировать, и в данном случае out параметр вычислится(инициализируется) первым. Вроде как и в C++ порядок не определен и вылазиют разные проблемы
кхм, я не спец по дельфям, но набросал у себя в блокноте следующее (пусть будет и здесь, чтобы потом сравнить с правильными ответами):

Задача 1:
не знаю, но подозреваю:
уж не будет ли выполнен OutData.Release() в функции AddData() до вызова функции CreateMerged_IData() ???
(что-то мне звучит бредово, но других мыслей нет, да и когда в дельфях не было досадных багов и/или недоработок? по-хорошему, надо в дизассемблере это предположение проверять)

Задача 2:
После выполнения obj.Free() переменная obj сохраняет значение (obj <> nil), и в старых дельфях код должен работать как часы…
Но как быть с современными версиями дельфей, которые умеют «автоматический подсчет ссылок» ???
( инструкция «obj := nil» по сути есть «пинок» нынешним дельфям для выполнения «obj.Release()», но память obj^ к этому моменту может быть уже невалидной! разве нет? )

P.S.:
отчаянно не хватает информации о версии Delphi, на которой код работает неправильно ))
отчаянно не хватает информации о версии Delphi, на которой код работает неправильно ))

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

Нет. Нынешние не умеют считать ссылки для объектов. Иначе бы это сильно поломало обратную совместимость.
Уточнили в личке с yizraor, действительно с XE7 появился AUTOREFCOUNT для Android платформ. В задаче считаем что AUTOREFCOUNT отключен.
ой, ну да, вторая задача по-любому только на современных версиях дельфей работать будет, там же генерик используется ))
что-то сразу и не обратил внимания на генерик )
2) С учётом, что TObject.Free ничего страшного не делает, остается вариант, что в FCollection.Remove(obj) приходит нулевой указатель, а из дельфийского папира вроде как выходит, что значение nil недопустимо
Так как у камрадов появился вопрос: «как в Remove может попасть nil, если перед этим стоит Obj.Free, оно же должно крешится на этом».
Это не так:
var Obj: TObject;

Obj := nil;
Obj.Free;
не вызовет никаких «крешей». Возможно, что именно эта строчка — NIL.Free рассматривается как неочевидное и невероятное.
спасибо!
очень интересные ответы. особенно по первой задаче )
я некоторых тонкостей не знал в плане передачи интерфейсных параметров, а про «out-» и вовсе не подозревал, как именно там устроено.
буду знать… и для надежности сохранил страничку на жесткий диск )
Пожалуйста. Жаль только, что в целом статью отхабрили.
в смысле? это значит, что плюсов нету?
ну, сорри, я бы плюсанул, но у меня «недостаточно кармы для голосования» ((
В текущий момент минусов больше чем плюсов. Я за плюсами не гоняюсь, просто когда статья набирает хоть что-то — видно что народу интересно, а сейчас я не вижу смысла писать еще подобные статьи про «подводные камни».
p.s. А вообще давайте заканчивать говорить про плюсы/минусы. Тут такое не в почете, да и сам я это не люблю. Просто сделал для себя выводы касательно этой тематики статей.
ясно…
жаль, что выводы не особо положительные.
мне подобные темы всегда интересны, особенно в отношении языков, которыми пользуюсь
Only those users with full accounts are able to leave comments. Log in, please.