EAccessViolation mit StringListe

[Edelfix]

Hallo.

Ich habe eine Situation die ich leider nicht nachvollziehen kann.
Es geht um eine Access Vialotion in eine Quelcode Zeile die nur ab und zu kommt.
Ich konnte den Fehler noch nicht nachstellen. Nur Nachträglich im LogFile auslesen.

Die abgekürzte fassung der Funktion:

Delphi-Quellcode:

Function TDMAng.CheckRV(RD:TStringList;var errlist:TStringList):boolean;
begin
  result:=true;
  if not assigned(errlist) then errlist:=TStringList.Create;
  errlist.Clear;
  
  //viele Prüfungen ...
  
  if bPasstNicht then
    errlist.Add('"215","'+'Hinweis: '+'Passt nicht!')

  //viele Prüfungen...
   
  result:= errlist.count=0;
end;

Die Fehlermeldung:

madExcept version : 4.0.12
callstack crc : $00000000, $48d61ed6, $48d61ed6
exception number : 1
exception class : EAccessViolation
exception message : Zugriffsverletzung bei Adresse 00000000. Lesen von Adresse 00000000.

Dei Fehlerzeile ist mal "errlist.Clear;" oder "result:= errlist.count=0;".

Ich vermute das die Prüfung "if not assigned(errlist)" manchmal nicht greift.

Kann man die Funktion irgendwie optimieren?

[himitsu]

Der gezeigte Code-Teil an sich kann keinerlei Exceptions erzeugen, mit Ausnahme einer Einzigen, welche sich EOutOfMemory nennt.

Zitat von Edelfix:

Ich vermute das die Prüfung "if not assigned(errlist)" manchmal nicht greift.

Kann man die Funktion irgendwie optimieren?

Tja, dann hat wohl jemand außerhalb das errlist nicht orgendlich initialisiert, was man dann natürlich auch außerhalb beheben sollte, da es intern niemals einen sicheren Weg geben kann, um einen "defekten" Zeiger zu erkennen.



Pssst, ich glaub in deinem errlist.Add fehlt ein ", aber für den Fehler hier ist es egal.

Und bezüglich result:=true; sollte der Compiler auch was sagen, falls sich nicht irgendwo ein Exit; versteckt, aber auch erstmal egal.

[jaenicke]

Wenn errlist manchmal einen ungültigen Zeiger enthält, liegt das vermutlich daran, dass diese übergebene Liste irgendwo mit Free freigegeben, die Variable aber weiter verwendet wird.

Wenn du dort stattdessen FreeAndNil benutzt, funktioniert auch die Prüfung mit Assigned.

Allerdings stimmt architektonisch dort etwas nicht, wenn manchmal, aber nicht immer, Instanzen in einer solchen Routine erzeugt werden. Für Erstellung und Freigabe muss immer jemand konkret zuständig sein, am besten in der gleichen Ebene (sprich z.B. nicht in einer Klasse die Erzeugung und in einer anderen die Freigabe).

[Jasocul]

Delphi-Quellcode:

//Function TDMAng.CheckRV(RD:TStringList;var errlist:TStringList):boolean;
Function TDMAng.CheckRV(RD:TStringList;errlist:TStringList):boolean;
begin
  result:=true;
  if not assigned(errlist) then errlist:=TStringList.Create;
  errlist.Clear;
  
  //viele Prüfungen ...
  
  if bPasstNicht then
    errlist.Add('"215","'+'Hinweis: '+'Passt nicht!')

  //viele Prüfungen...
   
  result:= errlist.count=0;
end;

Erstmal das "var" beim Prozedurkopf für errlist rausnehmen. Das könnte das Problem schon beheben.
Ansonsten den Debugger nutzen und prüfen, wie die Inhalte sind und wo diese sich ändern und damit eine AV auslösen.

EDIT:
Den Unsinn mit dem "var" bitte ignorieren.

[himitsu]

Zitat von Jasocul:

Erstmal das "var" beim Prozedurkopf für errlist rausnehmen. Das könnte das Problem schon beheben.

Dann hast du aber 100%ig in Zeile 2 ein Speicherleck.
Nein, wenn der Zeiger ungültig ist, dann bleibt er das auch, egal ob mit oder ohne Var, da sich innerhalb dieser Funktion rein garnichts ändert.

Und was jaenicke meint:
* entweder jemand in der gleichen Klasse kümmert sich um die Freigabe, oder garnichts erstellen
* oder wenn errlist Assigned ist, dann die Fehler loggen, bzw. ansonsten nur der Boolean-Result

[Jasocul]

Zitat von himitsu:

Zitat von Jasocul:

Erstmal das "var" beim Prozedurkopf für errlist rausnehmen. Das könnte das Problem schon beheben.

Dann hast du aber 100%ig in Zeile 2 ein Speicherleck.

Asche auf mein Haupt. Du hast natürlich Recht.

[SebastianZ]

Zitat von Edelfix:

Hallo.

Ich habe eine Situation die ich leider nicht nachvollziehen kann.
Es geht um eine Access Vialotion in eine Quelcode Zeile die nur ab und zu kommt.
Ich konnte den Fehler noch nicht nachstellen. Nur Nachträglich im LogFile auslesen.

Die abgekürzte fassung der Funktion:

Delphi-Quellcode:

Function TDMAng.CheckRV(RD:TStringList;var errlist:TStringList):boolean;
begin
  result:=true;
  if not assigned(errlist) then errlist:=TStringList.Create;
  errlist.Clear;
  
  //viele Prüfungen ...
  
  if bPasstNicht then
    errlist.Add('"215","'+'Hinweis: '+'Passt nicht!')

  //viele Prüfungen...
   
  result:= errlist.count=0;
end;

Wie schon gesagt wurde, wird hier die Liste mit free vorderbei freigegeben.
Ich würde die Liste nicht als Var-Parameter übergeben. Dadurch ist nicht mehr ersichtlich wer die Hoheit über die Liste hat, und wer sie freizugeben hat.

Sauberer wäre aus meiner Sicht, du baust eine lokale Liste auf, gibst nur den String zurück, und fügst den String dann in die eigentliche Liste ein.

Also so in etwa:

Delphi-Quellcode:

function CheckRV(RD:TStringList; out ErrorText: String): Boolean;
var
  ErrorList: TStringList;
begin
  ErrorList := TStringList.Create;
  try
    result:=true;
  
    //viele Prüfungen ...
  
    if bPasstNicht then
      ErrorList.Add('"215","'+'Hinweis: '+'Passt nicht!')

    //viele Prüfungen...
   
    result:= ErrorList.count=0;
    ErrorText := ErrorList.Text;
  finally
    FreeAndNil(ErrorList);
  end
end;


procedure foo;
var
  lstError: TStringList;
  ErrorText: String;
begin
  lstError := TStringList.Create;
  try
    if not CheckRV(dummy, ErrorText) then
      lstError.Text := lstError.Text + ErrorText;

  finally
    FreeAndNil(lstError);
  end

end;

[hoika]

Hallo,

Zitat:

Ich würde die Liste nicht als Var-Parameter übergeben. Dadurch ist nicht mehr ersichtlich wer die Hoheit über die Liste hat, und wer sie freizugeben hat.

100% Zustimmung.
Wer die Liste erzeugt, muss sie auch wieder freigeben.

Falls der Aufwand zu gross ist, dann sollte die Liste mit FreeAndNil freigegeben werden.

Zur Not hilft auch ein Singleton-Pattern,
um sicherzustellen, dass die StringListe wirklich nur einmal erzeugt wird.

[Edelfix]

Danke an alle.


Ich werde den Vorschlag von SebastianZ ümsetzen.

Ich denke auch das ein String als Var-Parameter unproblematischer ist.

Vielen Dank.