FreeAndNil und Pointer

[Mathias]

Sowas dürfte er nach meiner Meinung gar nicht kompilieren.

Code: Alles auswählen

uses
  SysUtils;

var
  p: PInteger;
begin
  new(p);
  FreeAndNil(p);

Da habe ich mir folgendes angeuckt.

Code: Alles auswählen

    procedure FreeAndNil(var obj);
      var
        temp: tobject;
      begin
        temp:=tobject(obj);
        pointer(obj):=nil;
        temp.free;
      end;

Wieso steht da nicht einfach

Code: Alles auswählen

procedure FreeAndNil(obj: TObject);

?

[Jorg3000]

Hi!
Deine Kritik ist berechtigt, aber es gibt dafür keine Lösung. Das wurde im englischen Forum schon lang diskutiert.
Es soll ein var-Parameter sein, damit die Variable wirklich auf NIL gesetzt wird. Dieses etablierte Verhalten lässt sich nachträglich nicht mehr ändern (Kompatibilität).
Mit dem vorgeschlagenen (var obj: TObject) funktioniert jedoch nicht, dass man einen beliebigen Objekt-Typ übergibt. Deshalb ohne eine Typenfestlegung, aber dadurch kann man leider alles übergeben.
Das ist kein FreePascal-Bug, sondern war auch in alten Delphi-Versionen schon immer so enthalten.

[Mathias]

Jetzt sehe ich das Problem, da motzt der Compiler.

Code: Alles auswählen

procedure NewFreeAndNil(var obj: TObject);
begin
end;

procedure TForm1.Button1Click(Sender: TObject);
var
  sl: TStringList;
begin
  sl := TStringList.Create;
  NewFreeAndNil(sl);
end;   

Ich bin heute zufällig auf das Problem gestossen, als ich eine Klasse zu einem Precord umbaute.

[PascalDragon]

Eine mögliche Lösung in 3.3.1 wäre das Folgende, würde aber benötigen, dass der benutzende Code mit $ModeSwitch ImplicitFunctionSpecialization kompiliert wird, damit man keine explizite Spezialisierung benötigt:

Code: Alles auswählen

generic procedure FreeAndNil<T: class>(var obj: T);
var
  tmp: T;
begin
  tmp := obj;
  obj := Nil;
  tmp.Free;
end;

[Jorg3000]

Hi!
Wäre es nicht auch denkbar, FreeAndNil() als Compiler Intrinsic umsetzen?
Dann könnte der Compiler unabhängig von der Definition einer RTL-Funktion prüfen, ob es irgendein TObject-Nachfahre ist.
Außerdem macht die kleine Funktion ja wirklich nicht viel, d.h. man könnte es in Assembler als äußerst schlankes Inline umsetzen, anstelle eines Funktionsaufrufs.

[Mathias]

Wäre es nicht auch denkbar, FreeAndNil() als Compiler Intrinsic umsetzen?

Ich vermute dies passt nicht zu Konzept von Pascal.

Was ich mich eher frage, wieso wurde nicht hier ein "self := nil" eingefügt.

Code: Alles auswählen

      procedure TObject.Free;

        begin
           // the call via self avoids a warning
           if self<>nil then
             self.destroy;
        end;   

[m.fuchs]

Was ich mich eher frage, wieso wurde nicht hier ein "self := nil" eingefügt.

Code: Alles auswählen

      procedure TObject.Free;

        begin
           // the call via self avoids a warning
           if self<>nil then
             self.destroy;
        end;   

Weil dann self auf nil gesetzt wird, aber nicht die Objektvariable. Damit ist keinem geholfen.

[PascalDragon]

Wäre es nicht auch denkbar, FreeAndNil() als Compiler Intrinsic umsetzen?

Technisch machbar ja, gewünscht absolut nicht. Es sollte nur das Compilermagic sein, was wirklich Compilermagic sein muss und FreeAndNil muss das definitiv nicht sein.

Wäre es nicht auch denkbar, FreeAndNil() als Compiler Intrinsic umsetzen?

Ich vermute dies passt nicht zu Konzept von Pascal.

Was ich mich eher frage, wieso wurde nicht hier ein "self := nil" eingefügt.

Code: Alles auswählen

      procedure TObject.Free;

        begin
           // the call via self avoids a warning
           if self<>nil then
             self.destroy;
        end;   

Die Deklaration einer Methode ist letztlich wie folgt:

Code: Alles auswählen

procedure TObject_Free(Self: Pointer);

Jetzt überleg mal, warum es nichts bringt Self auf Nil zu setzen.

[Jorg3000]

Dass Mathias fragt, warum man nicht einfach self:=nil; setzen kann, finde ich absolut nachvollziehbar.
Denn nur anhand von TObject.Free kann man nicht sehen, wo das self in der Procedure überhaupt herkommt (bei allen Methoden im Allgemeinen). Das self ist einfach immer da.
Ich habe früher auch gedacht, self wäre eine Referenz auf die aufrufende Variable. Habe erst später gemerkt, dass es wohl nur eine lokale Variable ist.
Vermutlich wird es in irgendeiner Dokumentation irgendwo erwähnt, aber offensichtlich ist es nicht.

[Warf]

Man sollte sowieso überlegen ob man wirklich FreeAndNil verwenden will, denn grundsätzlich versteckt es Fehler:

Code: Alles auswählen

sl:=TStringList.Create;
sl.Free;
sl.Free;

Doppeltes free ist ganz klar ein Fehler, und normalerweise eine Indikation das irgendwo im Code falsche Annahmen über die Lebenszeit des Objekts getroffen wurden (z.b. wenn's keinen eindeutigen owner gibt).

Und wenn man einen Fehler im Code hat will man das es kracht, zum einen damit das Programm nicht im Fehlerstate weiter läuft (und potentiell noch mehr Schaden anrichtet) und damit man einen stack trace und debug Infos bekommt was wo schief gegangen ist.

Der Code oben crasht mit einem Fehler, das ist gut. Ersetzt man den Aufruf zu Free jetzt durch FreeAndNil crasht es nicht mehr, der Fehler bleibt unentdeckt und es geht womöglich noch mehr kaputt:

Code: Alles auswählen

sl:=TStringList.Create;
FreeAndNil(sl);
FreeAndNil(sl);

Mal ganz davon abgesehen das in den allermeisten Fälle FreeAndNil eh nix bringt da die meisten Fälle von use after free die FreeAndNil verhindern soll, kommen dadurch zustande das man mehrere Referenzen auf die selbe Instanz hat:

Code: Alles auswählen

sl:=TStringList.Create;
sl2:=sl;
FreeAndNil(sl);
if assigned(sl2) then WriteLn(sl2.text);

Hier macht FreeAndNil nix.

Also zusammengefasst: FreeAndNil verhindert use-after-free in den meisten nicht trivialen Programmen sowieso nicht, und in den Fällen in denen es das tun kann, kann es Fehler sogar verstecken.

Wenn man Speichersicherheit sicherstellen will gibt es bessere Methoden dafür, z.b. Nutzung eines strikten ownership Modells (wie das component system), Verwendung von managed Typen wie Interfaces, oder Records, etc.
Und natürlich eins: testen, testen, testen...
Insbesondere mit Heaptrc (Tipp: KeepReleased) oder Valgrind

[Mathias]

Man sollte sowieso überlegen ob man wirklich FreeAndNil verwenden will, denn grundsätzlich versteckt es Fehler:

Ich mache es eigentlich auch lieber so, da sieht man sofort was passiert.

Code: Alles auswählen

  p := TObject.Create;
  if p <> nil then begin
    p.Free;
    p := nil;
  end;

Und bei einfachen Sachen, reicht ein.

Code: Alles auswählen

  p := TObject.Create;
  // mache etwas mit p
  p.Free;

Bei Assigned(p) fragt man sich auch immer wieder was es macht.
So nebenbei habe ich gerade gemerkt, das assigned ein ntrinsic ist.

[Warf]

Wenn ich mich richtig erinnere ist assigned notwendig in mode Delphi, da es hier keinen Unterschied zwischen funktionspointern und aufrufen gibt:

Code: Alles auswählen

if FunktionDiePointerReturned <> nil then // ruft Funktion auf
if Assigned(FunktionDiePointerReturned) then // nimmt den funktionspointer der Funktion 

Und übrigens, create kann nie nil zurückgeben, es kann eine exception werfen, aber dann ist der Rückgabewert undefiniert.

Und Bezüglich Pointer auf nil setzen, das mache ich ausschließlich wenn der danach nochmal benutzt wird und explizit nil sein soll.

Bei tests aktiviere ich meistens Heaptrc.KeepReleased:=true, damit werden Objekte bei dem free geflagged (mit dem bestimmten Speicher pattern gefüllt) statt zu freen, und wenn ich dann Versuche drauf zuzugreifen krachts und ich finde den Fehler einfach und schnell.
Beim setzen auf nil können Funktionen die mit nil funktionieren (wie free) sonst den use-after-free schlucken ohne zu crashen.

Das problem ist tatsächlich eher das Free einfach nil schluckt, auch wenn man das in den meisten Fällen eigentlich nicht will. In den Fällen sollte man eigentlich destroy aufrufen statt free

[m.fuchs]

Ich habe früher auch gedacht, self wäre eine Referenz auf die aufrufende Variable.

Aber auch dann würde ja nur die Referenz auf nil gesetzt werden.

Das ist in der Tat ein bisschen komplexer und ich bin am Anfang auf ein paar Mal reingefallen, weil mir da die Erfahrung und das Wissen gefehlt hat.

[Mathias]

Wenn ich mich richtig erinnere ist assigned notwendig in mode Delphi, da es hier keinen Unterschied zwischen funktionspointern und aufrufen gibt:

Stimmt, dieser unglückliche Fall gibt es.

Code: Alles auswählen

program Project1;

  function Test: Pointer;
  begin
    WriteLn('function');
    Result := Pointer(1234);
  end;

var
  t: function: Pointer;

begin
  t := @Test;
  WriteLn(PtrUInt(t));      // -> irgendwas
  WriteLn('------');
  WriteLn(PtrUInt(t()));  // -> 1234
end.      

Da muss man die Function in C-Konform aufrufen.

[PascalDragon]

Ich habe früher auch gedacht, self wäre eine Referenz auf die aufrufende Variable. Habe erst später gemerkt, dass es wohl nur eine lokale Variable ist.

Es ist ein versteckter Parameter, keine lokale Variable.

Vermutlich wird es in irgendeiner Dokumentation irgendwo erwähnt, aber offensichtlich ist es nicht.

Es wird nicht direkt im Zusammenhang mit Methoden erwähnt, aber indirekt im Zusammenhang mit statischen Klassenmethoden:

FPC knows static class methods in classes: these are class methods that have the Static keyword at the end. These methods behave completely like regular procedures or functions. This means that:

• They do not have a Self parameter. As a result, they cannot access properties or fields or regular methods.
• They cannot be virtual.
• They can be assigned to regular procedural variables.

Man sollte sowieso überlegen ob man wirklich FreeAndNil verwenden will, denn grundsätzlich versteckt es Fehler:

Ich mache es eigentlich auch lieber so, da sieht man sofort was passiert.

Code: Alles auswählen

  p := TObject.Create;
  if p <> nil then begin
    p.Free;
    p := nil;
  end;

Wie Warf erwähnt hat: Der Konstruktor einer Klasse gibt nie Nil zurück, sondern wirft bei Problemen eine Exception. Das heißt deine if-Bedingung ist implizit immer wahr, da sie im Fall einer Exception nicht erreicht werden wird.

Und bei einfachen Sachen, reicht ein.

Code: Alles auswählen

  p := TObject.Create;
  // mache etwas mit p
  p.Free;

Selbst dann sollte man besser einen Ressourcenschutzblock nehmen, da beim Arbeiten mit p eine Exception auftreten könnte.