Schutzverletzung beim Freigeben einer TStringList

[Garfield]

Mit lkVCDxRip.exe wird eine SVCD gerippt/ein SVCD-Image extrahiert. Anschließend werden mit folgendem Quelltext aus der XML die Kapitelzeiten ausgelesen und in einer Textdatei gespeichert.

Delphi-Quellcode:

  sl := TStringList.Create;  // Stringlist für das XML
  kl := TStringList.Create;  // Stringlist für die Kapitelzeiten

  sl.LoadFromFile(xml);  // Laden der XML

  for i := 0 to sl.Count - 1 do                  // Suchen der Tags mit den Kapitelzeiten
  if pos ('entry id=', sl.Strings[i]) > 0
  then begin
    von    := pos ('>', sl.Strings[i]);          //  Ende des Anfang-Tags
    bis    := pos ('</', sl.Strings[i]);         //  Anfang des Ende-Tags
    if bis > von
    then begin
      kap    := copy(sl.Strings[i], von + 1, bis - von - 4);  // Zeit in Kapitelliste kopieren
      kl.Add (kap);
    end;
  end;

  sl := NIL; // Stringlist des XML freigeben
  sl.Free;

  if kl.Count > 0  // Punkt gegen Komma austaschen und den Wert Null entfernen
  then begin
    i := 0;
    while i < kl.Count
    do begin
      kap           := Tausch(kl.Strings[i], '.', ',' ,false, true);
      kl.Strings[i] := kap;
      if StrToFloat(kap) = 0
      then kl.Delete(i)
      else inc(i);
    end;
  end;

  if kl.Count > 0  // Zeitformat ändern
  then begin
    for i := 0 to kl.Count - 1
    do begin
      kap := kl.Strings[i];
      kl.Strings[i] := FormatSecS(kap);  // Funktion in einer anderen Unit
    end;
  end;

  if kl.Count > 0  // Die Kapitelliste speichern
  then begin
    kap := (ChangeFileExt(Ziel, '.Kapitel'));  // Ziel ist die *.mpg von der SVCD
    kl.SaveToFile(kap);
  end;

  while not FileExists(kap)
  do sleep(50)

  kl := NIL;  // Kapitelliste freigeben
  kl.Free;

Beim Freigeben der Kapitelliste (kl.free) gibt es eine Schutzverletzung. Durch die Abfrage, ob die Kapiteldatei angelegt wurde, hat sich die Wahrscheinlichkeit, dass die Schutzverletzung auftritt, verringert. Wenn ich noch ein 'Sleep(1000);' verwende, tritt bei mir die Schutzverletzung nicht mehr auf. Allerdings bei anderen Anwendern.

Kann mir jemand erklären, warum an dieser Stelle eine Schutzverletzung auftritt und ob es eine bessere Lösung als ein Sleep gibt?

[alzaimar]

Die Zeilen

Delphi-Quellcode:

kl := Nil;
kl.Free;

Sind falschrum, dreh Sie um und es wird ein Schuh draus, oder verwende gleich
FreeandNil (kl);

[ManuMF]

und

Delphi-Quellcode:

sl := TStringList.Create;
try
  sl.LoadFromFile(...);
  ...
  ...
finally
  FreeAndNil(sl);
end;

Natürlich mit beiden.

Gruß,
ManuMF

[alzaimar]

Hupsa, selbstverständlich, ManuMF
[edit] [/edit]

[Garfield]

Ich danke Euch.

Ist schon eigenartig. In meinem Programm stehen die beiden Befehle noch einige Male in dieser falschen Reihenfolge, aber nur an der einen Stelle kommt es zu einem Fehler.

[tigerman33]

Dann werden die anderen Zeilen durch if-Abfragen o.ä. vor dem Ausführen "geschützt". Sonst knallt das nämlich 100%ig. => alle unbedingt ändern.

[Garfield]

Einige werden maximal einmal und andere fast immer aufgerufen. So wie bei sl. Und es gab bisher keine Fehler. Habe es jetzt alles in FreeAndNil geändert. Schließlich ist Vorsicht die Mutter der Prozellankiste.

[alzaimar]

Free ist eine Class Procedure, die erst prüft, ob die Referenz <> nil ist, und erst dann ggf. den destructor aufruft. Also, eigentlich dürfte auch hier nichts passieren:

Delphi-Quellcode:

Var
  sl : TStringlist;

Begin
  sl := nil;
  sl.Free;
End;

Wenn aber doch, dann tippe ich, ehrlich gesagt, auf einen dieser fiesen Seiteneffekte, der sich dann einstellt, wenn irgendwo im Code was zerschossen wurde (Durch Überschreiben eines freigegebenen Speicherbereiches etwa).

[GuenterS]

Hallo, mir ist aufgefallen, dass Du die kl StringListe öfter als notwendig durchgehst (bzw. brauchst Du sie gar nicht durchgehen), habe deine procedure dahingehend überarbeitet, dass dies nicht passiert und auch schon die try ... finallys eingebaut.

Delphi-Quellcode:

procedure TueWas(xml, Ziel: string);
var
  sl, kl: TStringList;
  kap: string;
  i, von, bis: integer;
begin
  kl := TStringList.Create; // Stringlist für die Kapitelzeiten
  try
    sl := TStringList.Create; // Stringlist für das XML
    try
      sl.LoadFromFile(xml); // Laden der XML

      for i := 0 to sl.Count - 1 do // Suchen der Tags mit den Kapitelzeiten
        if pos('entry id=', sl.Strings[i]) > 0 then
        begin
          von := pos('>', sl.Strings[i]); //  Ende des Anfang-Tags
          bis := pos('</', sl.Strings[i]); //  Anfang des Ende-Tags
          if bis > von then
          begin
            kap := copy(sl.Strings[i], von + 1, bis - von - 4);
            kap := StringReplace(kap, '.', ',', rfReplaceAll);
            if StrToFloat(kap) <> 0 then
              kl.Add(FormatSecS(kap)); // Zeit in Kapitelliste kopieren
          end;
        end;
    finally
      sl.Free; //StringList des XML freigeben
    end;

    if kl.Count > 0 then {// Die Kapitelliste speichern }
    begin
      kap := (ChangeFileExt(Ziel, '.Kapitel'));
        // Ziel ist die *.mpg von der SVCD
      kl.SaveToFile(kap);
    end;

  finally
    kl.Free;
  end;
end;

[tigerman33]

@Alzaimar:
Free ist dazu da, den von der Instanz alloziierten Speicher wieder freizugeben. Wenn man vorher den Objektzeiger auf nil setzt, ist ein Aufruf von Free im besten Fall unsinnig und der Speicher ist halt verloren, oder es knallt.