Italian community of Lazarus and Free Pascal

Programmazione => Generale => Topic aperto da: DragoRosso - Aprile 03, 2022, 01:41:59 pm

Titolo: DA NON FARE ... FREE INTERNO ALLA CLASSE
Inserito da: DragoRosso - Aprile 03, 2022, 01:41:59 pm
Riporto un argomento discusso in un forum internazionale di Delphi https://en.delphipraxis.net/topic/6536-why-isnt-this-dangerous-or-is-it/ (https://en.delphipraxis.net/topic/6536-why-isnt-this-dangerous-or-is-it/), argomento che sembra idiota ma non lo è assolutamente. L'ho portato su Lazarus e l'effetto è lo stesso.

Parto dal fatto che io non mi sarei mai sognato di fare una cosa simile, però effettivamente qualcuno potrebbe pensare che una cosa simile sia effettivamente conveniente, MA NON LO E' ... NON E' DA FARSI.

Questo il codice, una FORM con un pulsante e una nuova classe definita:

Codice: [Seleziona]
unit Unit1;

{$mode objfpc}{$H+}

interface

uses
  Classes, SysUtils, Forms, Controls, Graphics, Dialogs, StdCtrls;

type
  TDoSomething = class
    Danger: string;
    constructor create;
    procedure FaiQualcosa;
  end;

type

  { TForm1 }

  TForm1 = class(TForm)
    Button1: TButton;
    procedure Button1Click(Sender: TObject);
    procedure FormCreate(Sender: TObject);
  private

  public

  end;

var
  Form1: TForm1;
  DoSomething: TDoSomething;

implementation

{$R *.lfm}

constructor TDoSomething.create;
begin
  Danger := 'Danger!';
end;

procedure TDoSomething.FaiQualcosa;
begin
  ShowMessage('Faccio qualcosa');
  Sleep(1000);
  ShowMessage('Ho finito di fare qualcosa');
  Free; //<----- MAI FARE UNA COSA SIMILE !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
end;

procedure TForm1.Button1Click(Sender: TObject);
begin
  DoSomething.FaiQualcosa;
  if Assigned(DoSomething) then                 // Questo vale TRUE perchè il puntatore contiene il vecchio valore, che punta però a una zona orami "liberata" di memoria ........
    ShowMessage(DoSomething.Danger);    //<------  ERRORE ???????
end;

procedure TForm1.FormCreate(Sender: TObject);
begin
  DoSomething := TDoSomething.create;
end;

end.

In uno dei metodi della nuova classe è stato inserito il metodo FREE, con l'intento di "liberare" risorse della classe, ma ciò ha effetti deleteri.
Se eseguite quanto sopra, scoprirete che tutto viene eseguito ... più o meno correttamente senza evidenti errori ... salvo che l'ultimo ShowMessage che dovrebbe stampare la scritta "Danger!" stampa in realtà un riga vuota.
Anzi volendo proprio dire, LI NON DOVEVA ESSERE ESEGUITO NULLA O ANDARE IN ERRORE IL PROGRAMMA.

Cosa succede: la procedura FaiQualcosa esegue il "FREE" dell'istanza della classe, ma il puntatore, ossia la variabile "DoSomething"  mantiene ancora il valore dell'istanza in quanto l'istanza non "appartiene" a se stessa. Il contesto di creazione dell'istanza di TDoSomething non è l'isatnaza stessa e quindi l'istanza non può "liberarsi" da sola. In effetti io non sono convinto che il FREE esegua ciò che si pensa, in quanto alcune funzionalità legate alla creazione di un oggetto potrebbero non appartenere all'istanza.
Esempio banale è un ActiveX (oggetto OLE), è un oggetto che si "crea" ma lui non può in alcun modo "autodistruggersi" perchè il controllo e la sua "vita" dipende dal sistema operativo e da questo deve essere "distrutto".

Scusate se uso termini non appropriati all'informatica, ma è per enfatizzare il tutto.

L'istanza della classe TDoSomething, il cui puntatore viene mantenuto in DoSomeThing, non esiste (l'istanza è stata "liberata") ma il suo puntatore esiste ancora e non è NIL. Quindi tutto presegue come nulla fosse. Con un codice più complesso, se invece di uno ShowMessage con visualizzato un "vuoto" avessimo chiamato un metodo che esegue qualcosa magari su un DB o peggio .........

Se si imposta "usa la unit heaptrc" nelle opzioni di progetto, viene generato un errore a runtime in queste condizioni, ma basta cambiare leggeremente codice e neanche quel flag consente di intercettare il malfunzionamento.

Condizioni di questo tipo sono difficilissime da debugare, sopratutto dopo qualche mese dalla stesura del codice. Ricordatevi le regole di buona programmazione, tra cui "chi crea un oggetto anche lo distrugge". In questo caso la Form1 lo crea e la Form1 (e solo la Form1) deve distruggerlo.

Ciao
Titolo: Re:DA NON FARE ... FREE INTERNO ALLA CLASSE
Inserito da: quack - Aprile 03, 2022, 04:15:20 pm
Ciao DragoRosso,

ho voluto provare per curiosità e sul mio PC (Win10\Laz SVN\FPC 3.2.2) viene sollevata un'eccezione non appena il programma prova ad accedere alla stringa Danger, quindi comportamento corretto.

Con Delphi, ShowMessage mostra una riga vuota al primo tentativo e successivamente
solleva l'eccezione, quindi comportamento più infimo da debuggare.

Saluti
QK
Titolo: Re:DA NON FARE ... FREE INTERNO ALLA CLASSE
Inserito da: DragoRosso - Aprile 03, 2022, 05:28:12 pm
A me con Lazarus ultima release stabile (2.2 / 3.2.2 - WINDOWS x64, app x64) riporta come con Delphi una riga vuota. Abilitando heaptrc segnala un eccezione (ma non sempre). Anche la funzione Assigned(..) ogni tanto ritorna false e non esegue lo ShowMessage ...

Il comportamento è prettamente casuale, può generare o meno errore dipendentemente dal fatto che la memoria liberata sia o meno già "occupata" o da altri fattori. Può non generare errore per mille volte e la mille e una crashare, o vicerversa.

E' per questo che è "pericoloso" effettuare tali operazioni ... hai una casualità di eventi che poi tracciare è un problema.

Tanto tempo fà, ricordo un problema con la Close delle Form (in Delphi) .... non si poteva fare una Close da un evento della stessa Form, bisognava fare una Release. Poi l'hanno "sistemata". Quello era una "pensata" diabolica estrema, ma il concetto è equivalente.
Provate a pensare ad un Thread con settata la proprietà FreeOnTerminate a True (sinceramente non l'ho mai provata ne mai la proverò) : il thread si "autodistrugge" e ciò và bene se non è riferito ad una variabile, senza metodi ne proprietà aggiuntive; ma se fosse una istanza assegnata ad una variabile .... bhè allora sarebbe un problema perchè chiamare un qualsiasi metodo, proprietà o altro in qualsiasi istante potrebbe mandare in crash l'applicazione (perchè l'istanza potrebbe non esistere più).

Esattamente come eseguire il FREE nel cattivo esempio riportato (ripeto, tenuto conto che secondo me potrebbe essere che il FREE non svolga correttamente il suo lavoro).

Ulteriormente, occorre pensare all'uso che se ne farebbe di tale classe: la uso ad esempio per derivarla ? Auguri ....

Ciao

EDIT: non l'avevo provato .... ma se si preme il BUTTON1 più volte ... viene eseguito il codice FAIQUALCOSA ... che però non dovrebbe esistere più ....  :o :o :o :o ... altro che MELTDOWN e SPECTRE ....
Titolo: Re:DA NON FARE ... FREE INTERNO ALLA CLASSE
Inserito da: nomorelogic - Aprile 04, 2022, 09:17:45 am
argomento interessante, anche perché serve a chiarire meglio alcuni funzionamenti della programmazione ad oggetti
vorrei fare un paio di considerazioni su quelle penso siano buone pratiche di programmazione, proprio per evitare di addentrarsi in anfratti di questo genere :)

ok per il puntatore che punta ad una istanza che non esiste più, però guardando questo brano di codice
mi viene in mente che si è voluto forzare il comportamento con un errore nella scrittura del codice
Codice: [Seleziona]
procedure TForm1.Button1Click(Sender: TObject);
begin
  DoSomething.FaiQualcosa;
  if Assigned(DoSomething) then                 // Questo vale TRUE perchè il puntatore contiene il vecchio valore, che punta però a una zona orami "liberata" di memoria ........
    ShowMessage(DoSomething.Danger);    //<------  ERRORE ???????
end;

andrebbe chiaramente scritto in questo modo
Codice: [Seleziona]
procedure TForm1.Button1Click(Sender: TObject);
begin
  if Assigned(DoSomething) then begin
    DoSomething.FaiQualcosa;
    ShowMessage(DoSomething.Danger);
  end;
end;

non cambia chiaramente nulla, ma il test con Assigned va fatto prima di usare l'istanza :)

detto questo, che un'istanza si autodistrugga non è una buona pratica di programmazione in generale in quanto sicuramente non si è autocreata... bisognerebbe demandare al creatore la distruzione delle istanze

distruzione che sarebbe meglio fare sempre con FreeAndNil

nomorelogic

Edit:
FreeAndNil, serve appunto ad impostare il puntatore a nil dopo aver chiamato il destructor
https://wiki.freepascal.org/FreeAndNil
Titolo: Re:DA NON FARE ... FREE INTERNO ALLA CLASSE
Inserito da: DragoRosso - Aprile 04, 2022, 02:05:18 pm
@nomorelogic

Certamente hai fatto bene a precisare la situazione. Di fatto il controllo delle "esistenza" degli oggetti lo si dovrebbe fare sempre ... ma ad ogni metodo chiamato dovrebbe essere prefissato il controlo stesso .... codice illegibile, spreco di tempo sia in scrittura che in elaborazione.

EDIT: preciso, hai certamente ragione, ma farlo sempre su tutte le chiamate non è praticamente possibile....  ;)

Come hai indicato non serve a nulla, perchè tu testi la sussistenza dell'istanza alla primo metodo, ma al secondo metodo dove già l'istanza non esiste ??? ..........

Diciamo che la buona regola è che o si incapsula tutto in un "try ... except" o "try ... finally" o si fà il controllo con assigned, almeno per le parti più consistenti o critche.

Io sono abituato a testare con un "try ... except" o con un confronto a NIL tutti i passi dove un istanza viene creata. Se accade qualcosa di non previsto fermo tutto.
Poi lungo il codice metto l'Assigned dove svolgo operazioni critiche e il try except in tutte le altre.

Aprirò un nuovo topic sul forum per raccogliere le idee e le usanze dei membri del forum ... poi si potrebbe fare un articolo su questo: "regole basilari di buona programmazione ... quando il buon senso prevale sulla ragione"  ;D

Sul FreeAndNil, nel passato mi ha dato qualche grattacapo (in Delphi) e da allora sono abituato (come facevo prima) ad usare il Free seguito dal NIL.

Ciò è maggiormente vero quando esegue operazioni di "distruzioni di istanze" che appartengono ad esempio alla Form. Come sapete tutti gli oggetti usati a DesignTime, ma anche molti di quelli "creati" a RunTime vengono distrutti automaticamente alla Form (normalmente la principale) quando essa stessa si chiude .... io però per alcuni di essi non mi fido (tipicamente i DB, oggetti legati alle comunicazioni o simili) e li distruggo io a mano dentro il "distruttore" della Form. Mettendoli a NIL si evita che il programma dia errori in chiusura perchè trova un puntatore non NIL e cercando di eseguire il FREE crasha.

Ho notato che su molte istanze "asyncrone" (nel senso che ad esempio sono legate a Thread o altro) usare il FreeAndNil non è una operazione saggia.

Per i Thread puri io setto il TERMINATE a True, lancio un evento abbinato al Thread (molti Thread li ho con un WAIT infinito) ed eseguo un WaitFor, poi il Free e il NIL. Da quando faccio così non ho mai avuto alcun tipo di problema.

Comunque, da ripetersi per la terza volta (due io e un @nomorelogic): un istanza và distrutta nel contesto in cui viene istanziata. Se lo faccio nel Create della Form la distruggo nel Close (o nel Destroy) della Forma stessa, e così via. Seguite questa via e non avrete problemi.

Altra situazione è quella indicata nel mio precedente post in cui segnalo che una istanza "liberata" che non esiste più continua ad eseguire i metodi ... finchè la variabile esiste (non è a NIL) e l'area di memoria della ex classe istanziata non viene toccata, tutto funziona come nulla fosse ... il compilatore (e il loader del SO) ha fornito gli indirizzi e finchè questi non vengono toccati (su questo si basano una buona parte di attacchi HACKER) ...

Ecco, non ho mai provato a fare cose simili nè mi è mai accaduto neanche per sbaglio ma ritenevo che un minimo di "protezione" in realtà ci fosse, cioè che il FREE eseguisse quel minimo di "pulizia" sulla classe istanziata ... come non sò, però.....

Non si finisce mai di imparare e di soffrire.

Ciao