Speicher aufräumen

[photor]

Moin,

bei TList bin ich in letzter Zeit zu TObjectList über gegangen. Wenn ich das richtig verstanden habe, sollte da das Aufräumen (per default) doch eigentlich schon in der Klasse integriert sein, richtig?

Recht viele Leaks haben mit meinem Logger zu tun. Da wird zum Schluss wohl nicht alles richtig freigegeben. Ist natürlich auch blöd, wenn man bis zum Ende noch LogMessages generieren will!

Dann nutze/letne ich gerade Generics (TObjectList<…>). Ich denke, auch da liegt eine mögliche Quelle für Leaks.

Ciao,
Photor

[PascalDragon]

bei TList bin ich in letzter Zeit zu TObjectList über gegangen. Wenn ich das richtig verstanden habe, sollte da das Aufräumen (per default) doch eigentlich schon in der Klasse integriert sein, richtig?

Ja, solange du den OwnsObjects Parameter des Constructors nicht auf False setzt, ist das der Fall.

Dann nutze/letne ich gerade Generics (TObjectList<…>). Ich denke, auch da liegt eine mögliche Quelle für Leaks.

Auch die generische Objektliste verhält sich da wie TObjectList: OwnsObjects ist per Default auf True.

[photor]

Ja, solange du den OwnsObjects Parameter des Constructors nicht auf False setzt, ist das der Fall.

Default ist True, also OwnObjects. So hatte ich das auch gelesen.

Auch die generische Objektliste verhält sich da wie TObjectList: OwnsObjects ist per Default auf True.

So dachte ich auch. Allerdings sind doch einige Einträge in der heap.trc die in den generics enden. Ich werde weiter suchen (immer mal wieder).

Ciao,
Photor

[Warf]

Vielleicht vergessen die liste selbst zu freen?

[photor]

Möglich ist das. Damit probier ich gerade rum. Allerdings passiert es dann recht häufig, Dass dann ein SIGSEV kommt.

Ich habe den Verdacht, dass es einen ordentlichen Destructor braucht; bisher sind die in meinem Code noch Mangelware.

Also, wie sollte ein Destructor denn z.B. hier aussehen?

Code: Alles auswählen

TCollectiveEntry = class
    CyclesAcc, Cycles, Amplitude: TMyReal;
    constructor Create;
    constructor Create(ca, c, a: TMyReal);
  end;
  
  TCollective = class
  private
    FName: string;
    FNoEntries: integer;

    FCyclesMin: TMyReal;
    FCyclesmax: TMyReal;
    FAccCyclesMin: TMyReal;
    FAccCyclesMax: TMyReal;
    FAmplitudeMin: TMyReal;
    FAmplitudeMax: TMyReal;

  public
    ColEntries: specialize TObjectList<TCollectiveEntry>;
    Collective_Valid: Boolean;

    constructor Create;
    property CyclesMin: TMyReal read FCyclesMin write FCyclesMin;
    property CyclesMax: TMyReal read FCyclesMax write FCyclesMax;
    property AccCyclesMin: TMyReal read FAccCyclesMin write FAccCyclesMin;
    property AccCyclesMax: TMyReal read FAccCyclesMax write FACCCyclesMax;
    property AmplitudeMin: TMyReal read FAmplitudeMin write FAmplitudeMin;
    property AmplituesMax: TMyReal read FAmplitudeMax write FAmplitudeMax;

    property NoEntries: integer read FNoEntries write FNoEntries;
    property Name: string read FName write FName;

 end;
 
 
 // Class TCollectiveEntry
// ======================
constructor TCollectiveEntry.Create;
begin
  CyclesAcc := 0.0;
  Cycles := 0.0;
  Amplitude := 0.0;
end;

constructor TCollectiveEntry.Create(ca, c, a: TMyReal);
begin
  CyclesAcc := ca;
  Cycles := c;
  Amplitude := a;
end;


// Class TCollective
constructor TCollective.Create;
begin
  FCyclesMin := 0.0;
  FCyclesMax := 0.0;
  FAccCyclesMin := 0.0;
  FACCCyclesMax := 0.0;
  FAmplitudeMin := 0.0;
  FAmplitudeMax := 0.0;

  FNoEntries := 0;
  FName := 'No_Collective';
  Collective_Valid := False;

  ColEntries := specialize TObjectList<TCollectiveEntry>.Create;
end; 
 

Im Code dann

Code: Alles auswählen

function Read_Collective(FullFilename: string): TCollective;
var
   collective_sl: TStringList;
 [...]
  CollectiveEntry: TCollectiveEntry;
begin
  Result := TCollective.Create;  
  
  [...]
  
  for i := 0 to NoLines - 1 do
  begin
    CollectiveEntry := TCollectiveEntry.Create(cyclesacc, cycles, amplitude);

    Result.ColEntries.Add(CollectiveEntry);
  end; 
  
 end; 

und dann am Ende

Code: Alles auswählen

procedure TMainForm.FormClose(Sender: Tobject);
begin
  FreeAndNil(Collective);
  NewLogger.Free;
End;

Vielleicht weiß jemand, wie das am besten gelöst würde. Bin lern-bereit,

Photor

PS: das ist ein Beispiel aus dem Code heraus gelöst. Da passiert noch 'ne Menge zwischen.

[Warf]

Ganz einfach, du schaust in welchen Konstruktoren du Create aufrufst, und dann brauchst du für die einen destruktor mit Free.

In deinem beispiel ist das hier:

Code: Alles auswählen

constructor TCollective.Create;
begin
  [...] // ne ganze menge kram ohne .Create

  ColEntries := specialize TObjectList<TCollectiveEntry>.Create;
end;

Also solltest du einen destruktor haben der das wieder freed:

Code: Alles auswählen

destructor TCollective.Destroy; // in der class def als override definiert
begin
  ColEntries.Free;
  inherited Destroy;
end;

Um nachdenken zu vermeiden als faustregel: Alles was du im Konstruktor erzeugst muss im Destruktor zerstört werden.
Wenn Objekte einander benutzen dann musst du diese Abhängigkeit beim Terstören berücksichtigen, also am besten immer in umgekehrter Reihenfolge zum createn destroyen, dann bist du auf der sicheren Seite.

Du solltest generell dich immer an das Owner prinzip halten, also jedes Objekt hat einen owner, welcher sich um das zerstören kümmern soll. Ist der Owner ein anderes objekt, ist der punkt an dem du es zerstören musst fast immer der Destruktor. Ist der owner eine Funktion (also das objekt ist lokal innerhalb einer funktion), solltest du try-finally benutzen.
Was du auf gar keinen Fall haben darfst ist das ein objekt mehrere owner hat. Z.B. macht die LCL das sehr oft so das objekte kopiert statt referenziert werden:

Code: Alles auswählen

f := TFont.Create; // die aktuelle funktion ist der owner von f
try
  Label1.Font := f; // f an Label1 übergeben
finally
  f.Free; // weil wir owner sind zerstören wir es hier
end;

und TLabel.SetFont sieht dann (ungefähr) so aus:

Code: Alles auswählen

procedure TLabel.SetFont(const AValue: TFont);
begin
  Self.FFont.Assign(AValue); // TLabel ist der owner von FFont aber nicht von AValue, daher wird AValue in FFont reinkopiert, sodass danach mit AValue gemacht werden kann was man will
end;

Also zusammengefasst, an jeder stelle in deinem code sollte klar sein wer der owner eines bestimmten objekts ist. Falls dem nicht so ist, solltest du dein design eventuell überdenken, oder musst auf sowas wie referenzzählung zurückgreifen (z.B. mittels COM Interfaces)

[photor]

Danke Warf,

Das hilft tatsächlich Danke.

Die Sachen mit dem "inherited Destroy" war/ist mir nicht klar, ist aber offensichtlich wichtig (ohne hatte ich sowas schon da stehen).

Ciao,
Photor

[Socke]

Die Sachen mit dem "inherited Destroy" war/ist mir nicht klar, ist aber offensichtlich wichtig (ohne hatte ich sowas schon da stehen).

Mit inherited Destroy rufst du den Destructor der Elternklasse auf. Das funktioniert hier genau so wie in allen virtuellen/überschriebenen Methoden. Nur so kann von der Elternklasse allokierter Speicher wieder freigegeben werden.

[photor]

Ok. Also gehört das quasi in jede Destructor-Definition (im Prinzip ist ja irgendwo jede Klasse implizit von einer anderen abgeleitet)?

Ich schau mir meinen Code jedenfalls weiter an, und versuche weiter, die Leaks zu schließen.

Danke,
Photor

[Socke]

Ok. Also gehört das quasi in jede Destructor-Definition (im Prinzip ist ja irgendwo jede Klasse implizit von einer anderen abgeleitet)?

Ja. Der Destructor ist bereits in TObject als virtual deklariert. Somit muss hier immer inherited Destroy; aufgerufen werden.

[photor]

ich habe jetzt mal heaptrc (nach Wiki) eingeschaltet/aktiviert. Im leakview zeigt sich auch was:

Code: Alles auswählen

Total Mem allocated: 4016928
Leaking Mem Size: 33423
Leaking Blocks Count: 699

Kurzer Zwischenstand:

Code: Alles auswählen

Total Mem allocated: 8820991
Leaking Mem Size: 3088
Leaking Blocks Count: 90

sieht schon mal besser aus (mit sogar mehr Funktionalität im Programm). Jetzt kommt noch viel Detailarbeit. Die meisten Leaks kommen von Blocks, die an anderer Stelle frei gegeben werden müssen als sie allociert wurden.

Ciao,
Photor