String freigeben nur wo?

[EWeiss]

Mein Problem ist leider immer noch nicht gelöst. :oops:

EurekaLog!

Zitat:

--------------------------------------------------------------------------------------------------------------------------------------
|Methods |Details|Stack |Address |Module |Offset |Unit |Class |Procedure/Method |Line |
--------------------------------------------------------------------------------------------------------------------------------------
|+Leak #1: Type=UnicodeString: Ref count - 1, Content: "'WARNING: DO NOT CHANGE THE ORDER OF THE PROPERTIES!"; Total size=118; Count=1|
|------------------------------------------------------------------------------------------------------------------------------------|
|00000002|03 |00000000|005E8586|SK_Aero.dll |00008586|System | |InternalUStrFromPCharLen | |
|00000002|04 |00000000|0082C55B|SK_Aero.dll |0024C55B|uSkinConfig|TSkinConfig|FBuffin |976[12] |
|00000002|04 |00000000|0082ABEF|SK_Aero.dll |0024ABEF|uSkinConfig|TSkinConfig|GetConfiguration |308[5] |
|00000002|04 |00000000|008234C4|SK_Aero.dll |002434C4|uSkin |TSkinEngine|InitSkin |644[25] |
|00000002|04 |00000000|00833E1B|SK_Aero.dll |00253E1B|uMaster | |SKAERO_InitSkin |806[4] |
|00000002|03 |00000000|75213366|kernel32.dll|00013366|kernel32 | |BaseThreadInitThunk | |
|00000002|03 |00000000|77019900|ntdll.dll |00039900|ntdll | | (possible RtlInitializeExceptionChain+97)| |
|00000002|03 |00000000|770198CE|ntdll.dll |000398CE|ntdll | | (possible RtlInitializeExceptionChain+47)| |
|------------------------------------------------------------------------------------------------------------------------------------|

Das Problem liegt hier.
FBuffin

Wenn ich im Dialog von EurekaLog doppelkicke dann geht er mir in diese Zeile.

Delphi-Quellcode:

  try
    try
      while not eof(ParseFile) do
      begin
        ReadLN(ParseFile, sBuffer);
        AppendToLinkedList(nReading, sBuffer); // sBuffer soll einen Memoryleak produzieren.
        inc(nReading);
      end;
    except
      raise Exception.Create(SysErrorMessage(GetLastError));
    end;
  finally
    nReading := 0;
    CloseFile(ParseFile);
  end;

Das Problem ist nur, ich weis nicht wo ich ihn freigeben soll.
Theoretisch würde ich es so machen.

Delphi-Quellcode:

      while not eof(ParseFile) do
      begin
        ReadLN(ParseFile, sBuffer);
        AppendToLinkedList(nReading, sBuffer); // sBuffer soll einen Memoryleak produzieren.
        inc(nReading);
        // String freigeben nachdem er übergeben wurde
        ZeroMemory(Pointer(sBuffer), Length(sBuffer) * SizeOf(Char));
        sBuffer := '';
      end;

Nur dann lösche ich den String im Array dieser ist dann auch '' obwohl ich den String erst nach der Übergabe lösche.

Delphi-Quellcode:

procedure AppendToLinkedList(nReading: Integer; sBuffer: string);
begin

  New(FPBuffer);

  if nReading = 0 then
  Begin
    New(FToPBuffer);
    LineStart := FToPBuffer;
    LineStart^.Nr := 0;
  end;

  FPBuffer^.Nr := nReading;
  FPBuffer^.Str := sBuffer; //<< der ist dann nachdem löschen von sBuffer leer
  LineStart^.Max := FPBuffer^.Nr;
  FToPBuffer^.Ptr := FPBuffer;
  FToPBuffer := FPBuffer;
end;

Keine Ahnung wo ich den String sonst freigeben soll wenn nicht an angezeigter Position.

Edit:
Hier das gleiche!

Delphi-Quellcode:

procedure FormatINI(Filename: string);
var
  sBuffer: string;
  ParseFile: TextFile;
  StringList: TStringList;
  StringParse: string;

procedure AppendToLinkedList(nReading: Integer; sBuffer: string);
begin

  New(FPBuffer);

  if nReading = 0 then
  Begin
    New(FToPBuffer);
    LineStart := FToPBuffer;
    LineStart^.Nr := 0;
  end;

  FPBuffer^.Nr := nReading;
  FPBuffer^.Str := sBuffer;
  LineStart^.Max := FPBuffer^.Nr;
  FToPBuffer^.Ptr := FPBuffer;
  FToPBuffer := FPBuffer;
end;

begin
  StringList := TStringList.Create;

  Assignfile(ParseFile, Filename);
  reset(ParseFile);

  try
    try
      while not eof(ParseFile) do
      begin
        ReadLN(ParseFile, sBuffer);
        AppendToLinkedList(nReading, sBuffer);
        inc(nReading);
      end;
    except
      raise Exception.Create(SysErrorMessage(GetLastError));
    end;
  finally
    nReading := 0;
    CloseFile(ParseFile);
  end;

  FPBuffer := LineStart;
  while (FPBuffer.Nr <= LineStart.Max - 1) do
  begin
    FPBuffer := FPBuffer.Ptr;
    StringParse := LeftTrim(FPBuffer.Str);
    if Length(StringParse) <> 0 then
      StringList.Add(FPBuffer^.Str);

    if FPBuffer.Nr > 0 then
      if LeftStr(FPBuffer^.Str, 1) = '[' then
        StringList.Insert(StringList.IndexOf(StringParse), '');

    ZeroMemory(PWideChar(FPBuffer.Str), Length(FPBuffer.Str) * SizeOf(Char));
    FPBuffer.Str := '';
  end;

  StringList.SaveToFile(Filename);
  StringList.Free;

  Dispose(FPBuffer);
  FPBuffer := nil;
  LineStart := nil;

end;

siehe Shot.. Doppelklick auf FormatINI dann springt er in diese Zeile.

Delphi-Quellcode:

AppendToLinkedList(nReading, sBuffer);

gruss

[Uwe Raabe]

Das ZeroMemory ist hier wohl eher schädlich, da es die Referenzzählung der Strings umgeht. Ein Finalize(FPBuffer^) sollte da verträglicher sein.

Was mir auffällt ist, daß für jede Zeile im AppendToLinkedList ein New() aufgerufen wird, es aber nur ein einziges Dispose() gibt.

Außerdem vermisse ich die Initialisierung von nReading mit 0.

[EWeiss]

Zitat:

Zitat von Uwe Raabe (Beitrag 1425839)

Das ZeroMemory ist hier wohl eher schädlich, da es die Referenzzählung der Strings umgeht. Ein Finalize(FPBuffer^) sollte da verträglicher sein.

Was mir auffällt ist, daß für jede Zeile im AppendToLinkedList ein New() aufgerufen wird, es aber nur ein einziges Dispose() gibt.

Könntest du mir bitte an Hand meines Codes das korrigieren wenn du zeit dazu findest?

Zitat:

Außerdem vermisse ich die Initialisierung von nReading mit 0.

Delphi-Quellcode:

FPBuffer^.Nr := nReading;

Nun das ist schon gegeben..
nReading ist 0 beim start.
Aber ich kann es noch ändern wobei ich aber denke das es nicht nötig ist.
Wenn ich eine Variable als integer definiere dann ist automatisch wenn nichts anderes angegeben wird der erste wert = 0 oder liege ich da falsch?
In dem Fall muss ich nReading nicht extra 0 zuweisen.

Aber wenn das für dich besser ist? Kein Problem.

Delphi-Quellcode:

  try
    try
      nReading := 0;
      while not eof(ParseFile) do
      begin
        ReadLN(ParseFile, sBuffer);
        AppendToLinkedList(nReading, sBuffer);
        inc(nReading);
      end;
    except
      raise Exception.Create(SysErrorMessage(GetLastError));
    end;
  finally
    CloseFile(ParseFile);
  end;

gruss

[DasWolf]

Zitat:

Zitat von EWeiss (Beitrag 1425840)

Wenn ich eine Variable als integer definiere dann ist automatisch wenn nichts anderes angegeben wird der erste wert = 0 oder liege ich da falsch?

Falsch.

[Uwe Raabe]

Zitat:

Wenn ich eine Variable als integer definiere dann ist automatisch wenn nichts anderes angegeben wird der erste wert = 0 oder liege ich da falsch?

Das gilt nur bei globalen Variablen und Feldern in Klassen. Lokale Variablen enthalten zufällige Inhalte.

Ich würde es aber in jedem Fall vor der while-Schleife setzen, denn dann könnte man die procedure auch mehrmals aufrufen.


In dem Code habe ich sonst nur das Freigeben der Strings entfernt und eine Schleife zum Freigeben der linked List eingebaut. Vielleicht ist das MemoryLeak damit ja schon behoben.

Delphi-Quellcode:

procedure FormatINI(Filename: string);
var
  sBuffer: string;
  ParseFile: TextFile;
  StringList: TStringList;
  StringParse: string;

procedure AppendToLinkedList(nReading: Integer; sBuffer: string);
begin

  New(FPBuffer);

  if nReading = 0 then
  Begin
    New(FToPBuffer);
    LineStart := FToPBuffer;
    LineStart^.Nr := 0;
  end;

  FPBuffer^.Nr := nReading;
  FPBuffer^.Str := sBuffer;
  LineStart^.Max := FPBuffer^.Nr;
  FToPBuffer^.Ptr := FPBuffer;
  FToPBuffer := FPBuffer;
end;

begin
  StringList := TStringList.Create;

  Assignfile(ParseFile, Filename);
  reset(ParseFile);

  try
    try
      nReading := 0;
      while not eof(ParseFile) do
      begin
        ReadLN(ParseFile, sBuffer);
        AppendToLinkedList(nReading, sBuffer);
        inc(nReading);
      end;
    except
      raise Exception.Create(SysErrorMessage(GetLastError));
    end;
  finally
    nReading := 0;
    CloseFile(ParseFile);
  end;

  FPBuffer := LineStart;
  while (FPBuffer.Nr <= LineStart.Max - 1) do
  begin
    FPBuffer := FPBuffer.Ptr;
    StringParse := LeftTrim(FPBuffer.Str);
    if Length(StringParse) <> 0 then
      StringList.Add(FPBuffer^.Str);

    if FPBuffer.Nr > 0 then
      if LeftStr(FPBuffer^.Str, 1) = '[' then
        StringList.Insert(StringList.IndexOf(StringParse), '');
  end;

  StringList.SaveToFile(Filename);
  StringList.Free;

  { linked List freigeben }
  while LineStart <> nil do begin
    FPBuffer := LineStart;
    LineStart := FPBuffer.Ptr;
    { Bei Dispose kümmert sich der Compiler um das Finalize }
    Dispose(FPBuffer);
  end;
  { keine dangling Pointer hinterlassen }
  FPBuffer := nil;
  FToPBuffer := nil;

end;

[EWeiss]

danke dir Uwe..

Ich habe es vorher mal so versucht.

Delphi-Quellcode:

  if not Assigned(FPBuffer) then
    New(FPBuffer);
  //.....
  finalize(FPbuffer^);
  Dispose(FPBuffer);
  FPBuffer := nil;

  finalize(LineStart^);
  Dispose(LineStart);
  LineStart := nil;

Ist das gleiche Ergebnis wie bei dir.
Ich bin darauf gekommen weil du sagtest

Zitat:

Was mir auffällt ist, daß für jede Zeile im AppendToLinkedList ein New() aufgerufen wird, es aber nur ein einziges Dispose() gibt.

Es ist ja eigentlich nicht nötig jedes mal den FPBuffer neu zu erstellen wenn ich es einmal getan habe.
Doch ist nötig ;) Deshalb nehme ich deine Lösung. Danke noch mal.

Beide Varianten melden jetzt keinen Speicherleck mehr.

Frage mich nur warum beim doppelklick in dem EurekaLog Dialog auf diese zeile gesprungen wird.

Delphi-Quellcode:

AppendToLinkedList(nReading, sBuffer); // sBuffer soll einen Memoryleak produzieren.

das irritiert doch etwas denn sBuffer kann ich dort nicht einfach löschen.

Zitat:

Zitat:

Zitat von DasWolf (Beitrag 1425842)

Zitat:

Zitat von EWeiss (Beitrag 1425840)

Wenn ich eine Variable als integer definiere dann ist automatisch wenn nichts anderes angegeben wird der erste wert = 0 oder liege ich da falsch?

Falsch.

Nun pauschal kann man das nicht sagen siehe die Erklärung dazu von Uwe.

gruss

[Uwe Raabe]

Zitat:

Zitat von EWeiss (Beitrag 1425845)

Es ist ja eigentlich nicht nötig jedes mal den FPBuffer neu zu erstellen wenn ich es einmal getan habe.

Dann gibt es doch aber auch nur einen Record in der linked List, oder?

BTW, warum überhaupt eine linked List? Eine TList<PassenderRecordTyp> ist doch viel einfacher zu handhaben. Das Ptr Feld im Record wäre dann auch obsolet.

Zitat:

Zitat von EWeiss (Beitrag 1425845)

Frage mich nur warum beim doppelklick in dem EurekaLog Dialog auf diese zeile gesprungen wird.

Delphi-Quellcode:

AppendToLinkedList(nReading, sBuffer); // sBuffer soll einen Memoryleak produzieren.

das irritiert doch etwas denn sBuffer kann ich dort nicht einfach löschen.

Der geleakte Speicher wird halt genau an der Stelle zugewiesen. Der String wird dann dem Record-Feld zugewiesen, was den Referenzzähler erhöht. Die Freigabe von sBuffer gibt dann aber eben nicht den Stringspeicher frei, da dieser ja noch von dem Record-Feld referenziert wird. Erst wenn der Record mit Dispose freigegeben wird, erfolgt auch die Freigabe des Stringspeichers.

Eurekalog kann aber nur erkennen, wo der Speicher alloziert wurde und das ist eben in dieser Zeile.

[EWeiss]

Sorry Uwe war vorschnell geurteilt ;)
Habe es editiert.

Zitat:

Ist das gleiche Ergebnis wie bei dir.
Ich bin darauf gekommen weil du sagtest

Zitat:

Was mir auffällt ist, daß für jede Zeile im AppendToLinkedList ein New() aufgerufen wird, es aber nur ein einziges Dispose() gibt.

Es ist ja eigentlich nicht nötig jedes mal den FPBuffer neu zu erstellen wenn ich es einmal getan habe.
Doch ist nötig ;) Deshalb nehme ich deine Lösung. Danke noch mal.

Zitat:

BTW, warum überhaupt eine linked List? Eine TList<PassenderRecordTyp> ist doch viel einfacher zu handhaben. Das Ptr Feld im Record wäre dann auch obsolet.

Na ja NonVcl halt wenn nicht unbedingt nötig verwende ich die Win32API.

gruss

[Neutral General]

Zitat:

Zitat von EWeiss (Beitrag 1425849)

Zitat:

BTW, warum überhaupt eine linked List? Eine TList<PassenderRecordTyp> ist doch viel einfacher zu handhaben. Das Ptr Feld im Record wäre dann auch obsolet.

Na ja NonVcl halt wenn nicht unbedingt nötig verwende ich die Win32API.

TList<T> ist NonVCL

[EWeiss]

Zitat:

Zitat von Neutral General (Beitrag 1425851)

Zitat:

Zitat von EWeiss (Beitrag 1425849)

Zitat:

BTW, warum überhaupt eine linked List? Eine TList<PassenderRecordTyp> ist doch viel einfacher zu handhaben. Das Ptr Feld im Record wäre dann auch obsolet.

Na ja NonVcl halt wenn nicht unbedingt nötig verwende ich die Win32API.

TList<T> ist NonVCL

Du hast recht :)
Habe es nun mal so gemacht.
Und mit Uwes Hilfe wird jetzt alles korrekt frei gegeben.

Danke nochmals.

gruss

[EWeiss]

Hmm.. musste es doch nochmal ändern.
Es funktioniert nur beim ersten mal wird die gleiche Funktion nochmals aufgerufen dann kracht es mit invaliden Pointer.

Delphi-Quellcode:

  { linked List freigeben }
   while LineStart <> nil do begin
     FPBuffer := LineStart;
     LineStart := FPBuffer.Ptr;
     { Bei Dispose kümmert sich der Compiler um das Finalize }
     Dispose(FPBuffer);
   end;
   { keine dangling Pointer hinterlassen }
   FPBuffer := nil;
   FToPBuffer := nil;

so geht's.

Delphi-Quellcode:

  Count := LineStart.Max;
  for i := 0 to Count do
  begin
    FPBuffer := LineStart;
    LineStart := FPBuffer.Ptr;
    Dispose(FPBuffer);
  end;

  Dispose(LineStart);

  FPBuffer := nil;
  FToPBuffer := nil;
  LineStart := nil;

finde den Fehler bei deiner Variante leider nicht.

gruss

[Dennis07]

Warum benutzt du nicht einfach PChar für Strings, die du selber verwalten willst? Oder, meinetwegen, WideString.
Denn "String" ist halt referenzgezählt und wird selber erzeugt und freigegeben. Bei "PChar" hast du auch schon "StrNew" und "StrDispose".

[EWeiss]

[QUOTE=Dennis07;1425881]Warum benutzt du nicht einfach PChar für Strings, die du selber verwalten willst? Oder, meinetwegen, WideString.QUOTE]

Weil ich es nun mal auf string ausgelegt habe und ja die sind auch Unicodefähig zumindest ab Delphi 2009 glaube ich.
Und im Grunde genommen sind Strings in Delphi schon mächtig.
Warum soll ich mich dann mit PChar (PWideChar) rumschlagen wenn es auch so geht.

Sorry aber ich habe keine Lust jetzt meine Library deshalb wieder komplett umzumodeln.
Für Außenstehende ist das einfach zu erfragen warum machst du das nicht so oder so. ;)

Zitat:

Denn "String" ist halt referenzgezählt und wird selber erzeugt und freigegeben

referenzgezählt stimme ich zu.
selbst freigegeben.. nein sonst müsste ich mir nicht die Arbeit machen oder?

gruss

[Blup]

Zitat:

Zitat von EWeiss (Beitrag 1425885)

selbst freigegeben.. nein sonst müsste ich mir nicht die Arbeit machen oder?

Diese Arbeit ist überflüssig, wie kommt man auf so eine Idee?

Delphi-Quellcode:

...
sBuffer := '';

Reicht völlig aus.

Strings verhalten sich wie Interfaces, Stringvariablen wie Interfacevariablen.
Ein String belegt einen bestimmten Speicherbereich, fällt der Referenzzähler des Strings auf 0, wird dieser an den Speichermanager als frei gemeldet.
(Das heist nicht das der Speichermanager diesen auch sofort an Windows frei gibt oder mit Nullen überschreibt.)
Wird ein String sowohl einer Variablen als auch einer Liste zugewiesen, wird trotzdem nur einmal Speicher belegt. Nur der Referenzzähler erhöht sich bei jeder Zuweisung.
Wird der Speicherbereich gelöscht, verweisen natürlich auch beide auf den gelöschten Speicher. Das ist natürlich nicht erwünscht.

[EWeiss]

Zitat:

Reicht völlig aus.

Thema war schon abgeschlossen aber gut..

Eben nicht!
Denn so hatte ich es vorher weil ich das auch angenommen habe.

Aber!
Wie im ersten Beitrag schon erwähnt EurekaLog gibt mir dann diese Speicherlecks.
Wenn meine vorherige Herangehensweise den String zu löschen richtig ist\wäre dann taugt EurekaLog nichts :)

gruss