delphipascal

In Delphi, is it a correct approach to Free the Sender within an Event?

So, say we have a class like this:

  TFieldsReadyEvent = procedure(Sender: TObject; Code, AuthorizatedID: string) of object;
  TCatcherServer = class(TServer)
  private
    FOnFieldsReady: TFieldsReadyEvent;
    FCode: string;
    FAuthoID: string;
    FAuthorizationType: TAuthorizationType;
    function GetAuthorizationType: string;
    function GetCode: string;
    function GetEntityID: string;
  public
    property OnFieldsReady: TFieldsReadyEvent read FOnFieldsReady write FOnFieldsReady;
    property Code: string read GetCode;
    property EntityID: string read GetEntityID;
    property AuthorizationType: string read GetAuthorizationType;
    procedure Callback(Session: TIdContext; Req: TIdHTTPRequestInfo; Res: TIdHTTPResponseInfo); override;
  end;

Where the Callback procedure looks like this:

procedure TCatcherServer.Callback(Session: TIdContext;
  Req: TIdHTTPRequestInfo; Res: TIdHTTPResponseInfo);
begin
  inherited Callback(Session, Req, Res);
  if (Req.Params.Values['code'] <> '') and ((Req.Params.Values['shop_id'] <> '') or (Req.Params.Values['main_account_id'] <> '')) then
  begin
    Res.ResponseNo := 200;
    Res.ResponseText := '{"message":"continue on the application"}';
    FCode := Req.Params.Values['code'];
    if (Req.Params.Values['shop_id'] <> '') then
    begin
      FAuthorizationType := atShopAuthorization;
      FAuthoID := Req.Params.Values['shop_id'];
    end else
    begin
      FAuthorizationType := atMainAccountAuthorization;
      FAuthoID := Req.Params.Values['main_account_id'];
    end;

    FOnFieldsReady(Self, FCode, FAuthoID);
  end else
  begin
    Res.ResponseNo := 400;
    Res.ResponseText := '{"message":"Something went wrong."}';
  end;
end;

Is it a correct approach to have in my form an event handler that is able to Free the TCatcherServer instance? Example as follows:

{Form1 public declarations}
procedure FieldsReadyHandler(Sender: TObject; Code, AuthorizatedID: string);

procedure TForm1.Button1Click(Sender: TObject);
begin
  VarServer := TCatcherServer.Create;
  VarServer.Listen(7070);
  VarServer.OnFieldsReady := FieldsReadyHandler;
end;

procedure TForm1.FieldsReadyHandler(Sender: TObject; Code,
  AuthorizatedID: string);
begin
  ServerLog.Lines.Add(Code);
  ServerLog.Lines.Add(AuthorizatedID);
  ServerLog.Lines.Add((Sender as TCatcherServer).AuthorizationType);

  Sender.Free;
end;

TL;DR Is it the correct approach to use events to free the sender in cases like this (where after the event is triggered I got the data I needed and don't need the Sender anymore). If not, what's the correct way of doing so?

Solution

  • It is usually NOT safe to free the Sender from inside its own event. You don't know what else may still need to access the Sender after the event handler exits.

    Case in point - if the Sender in question is the object that owns the TIdHTTPServer which is triggering your Callback() method, then you would be trying to free the Sender, and thus free the TIdHTTPServer, while TIdHTTPServer is still waiting for the Callback() to exit. Since TIdHTTPServer is a multi-threaded component that fires its OnCommand... events in worker threads, destroying the TIdHTTPServer in its own event would cause a deadlock.

    If you must free the Sender, you should do so asynchronously so you give the caller/RTL extra time to finish using the Sender. For instance, by using TThread.Queue(nil, Sender.Free); (just be aware that TThread.Queue() runs its predicate in the main thread, so make sure your Sender's destruction is thread-safe in this example).