- changed status to resolved
Memoryleak in TObjectDataSet's DoPostRecord method
I found small memoryleak problem in TObjectDataSet's DoPostRecord method. When while posting inserted record to a dataset occurs any exception during setting property value, then newItem instance created for assignment data is not freed and this cause memory leak. It is because newItem is created but not assigned to the list what could ensure proper instance destroying. I think DoPostRecord method should looks like:
procedure TObjectDataSet.DoPostRecord(Index: Integer; Append: Boolean);
var
newItem: TObject;
freeOnException: Boolean;
sortNeeded: Boolean;
i: Integer;
field: TField;
fieldValue: Variant;
value: TValue;
prop: TRttiProperty;
begin
freeOnException := State = dsInsert;
if State = dsInsert then
newItem := TActivator.CreateInstance(fItemTypeInfo)
else
newItem := IndexList.Items[Index];
try
sortNeeded := False;
for i := 0 to ModifiedFields.Count - 1 do
begin
field := ModifiedFields[i];
if not sortNeeded and Sorted then
sortNeeded := FieldInSortIndex(field);
// Fields not found in dictionary are calculated or lookup fields, do not post them
if fProperties.TryGetFirst(prop, TPropertyFilters.IsNamed(field.FieldName)) then
begin
fieldValue := field.Value;
if VarIsNull(fieldValue) then
prop.SetValue(newItem, TValue.Empty)
else
if TValue.From<Variant>(fieldValue).TryConvert(prop.PropertyType.Handle, value) then
prop.SetValue(newItem, value); -- here an exception in property setting can be thrown...
end;
end;
if State = dsInsert then
if Append then
Index := IndexList.AddItem(newItem)
else
IndexList.InsertItem(newItem, Index);
except -- freeing newItem instance when something went wrong
if freeOnException then
newItem.Free;
raise;
end;
...
here is added try except block which allows to free item not assigned to the list when something went wrong during data assignment to the item or list.
I found this problem in Branch_2.0-rc.3 but probably exists also in older versions.
Comments (3)
-
repo owner -
reporter Thank you very much for correcting this. Don’t you think the code with adding item to the list also should be in try except section? Only when an item is successfully added to the list we can be sure that there will be no memory leak. While adding item to the list also could be an exception raised. Maybe I'm paranoid :-), but it happens sometimes.
-
repo owner You could have an event on the list that raises an exception - but at that point the item was already added to the list and must not be freed.
There could be that rare case of an exception happening when adding the item to the list but before it was successfully added but to be fair, if that happens you are in more serious trouble than leaking that object.
- Log in to comment
fixed
#394→ <<cset 551daecd14a8>>