Speicherleaks TMemoryStream in einem Objekt

[Delphi.Narium]

Dashier ist nicht als nachahmenswert gedacht:

Delphi-Quellcode:

// Die Funktion Get_ProduktReport_General liefert einen Stream zurück:
function TDBService.Get_ProduktReport_General (Kunde: string; Von, Bis: TDate): TStream;
var MxSQL: TMxSQL;
begin
   MxSQL := TMxSQL.Create(false);
   Try
      // Die Funktion GetF_Report_Produkte_General liefert ebenfalls einen Stream zurück:
      Result := MxSQL.GetF_Report_Produkte_General(Kunde, Von, Bis);
      Logic_Main.FehlerProzedure := 'Get_ProduktReport_General 2';
   Finally
      MxSQL.Free;
   End;
end;

Man kann sich damit in der Funktion Get_ProduktReport_General das Erstellen eines Streams schonmal sparen, da Result schon ein Stream ist, benötigt man keinen weiteren Stream als Zwischenschritt, zumal LStream nach dem Erstellen sowieso nicht mehr benötigt wird, sondern einen anderen Stream zugewiesen bekommt.

Beim gesamten Konstrukt sehe ich eine große Chance, Speicherlöcher zu erstellen.

In GetF_Report_Produkte_General wird LStream erstellt. Dann passiert einiges, aber nicht mit LStream. Dann wird geprüft, ob LStream was enhält, was nach dem Create eher wahrscheinlich ist.

Nachdem nun klar ist, dass LStream etwas enthält, wird LStream was anderes zugewiesen (Logic.GridToStream(MsCols, MsRows); ) Unklar ist, ob der Inhalt von LStream hier irgendeinen Einfluß haben könnte. Tippe mal auf: eher nicht.

Dem erstellten und auf Vorhandensein, aber nicht weiter genutzten, LStream wird also was anderes zugewiesen und das wird dann als Result zurückgegeben.

Bis hierher erscheint mir LStream als absolut überflüssig. Meiner Meinung nach kann LStream vollständig aus der Funktion entfernt werden und aus

Delphi-Quellcode:

if Assigned(LStream) then begin
  LStream := Logic.GridToStream(MsCols, MsRows);
end;
Result := LStream;

könnte Result := Logic.GridToStream(MsCols, MsRows); werden. Damit bliebe dann "nur noch" Result als Stream übrig, dessen Freigabe man später vergessen könnte. Die Zahl der möglichen Speicherlöcher könnte / wird durch Weglassen der LStreams in beiden Funktionen jedenfalls verringert werden. Glücklich erscheint mir das Konstrukt insgesamt jedoch nicht.

[himitsu]

Zitat:

if Assigned(LStream) then

Nicht NOT?

Free und FreeAndNil besitzen bereits ein eingebautes if Assigned(Self) then Destroy,
entweder es wird erstellt oder nicht (dann isses NIL, im Destructor), also kann man eigentlich immer einfach direkt Free auffrufen, egal ob Assigned oder nicht. (außer von außen wurden Fremdobjekte reingegeben und diese werden nicht geOwned)
(im Destructor und bei lokalen Variablen verwende ich aber eigentlich nie FreeAndNil, weil schreibfaul und Free per Codevervollständigung kommt ... FreeAndNil ist meistens aber auch unnötig / nicht falsch)