Is this code safe: Queued calls and [weak] interfaces

[Keesver 23]

Hello,

 

We are looking at safely handling queued calls. The problem is that under some circumstances the object used inside the queued method is freed before the method gets executed. This raises an exception. Can we use this construct to fix it:

 

type
  ITest = interface
    ['{10DD63EA-490E-45D5-9250-72AEB1FF6D19}']
    function GetName: string;
    procedure SetName(const Value: string);
  end;

  TTest = class(TInterfacedObject, ITest)
  protected
    FName: string;

    function GetName: string;
    procedure SetName(const Value: string);
  end;

  TObjectWithTest = class
  private
    FTest: ITest;

  public
    constructor Create(const AInterface: ITest);
    procedure   QueueCallSafe([weak]AInterface: ITest);
    function GetTest: ITest;
  end;

implementation

procedure TObjectWithTest.QueueCallSafe([weak]AInterface: ITest);
begin
  TThread.ForceQueue(nil, procedure begin
    if AInterface = nil then
      ShowMessage('nil') else
      ShowMessage('not nil');
  end);
end;

procedure TForm1.Button3Click(Sender: TObject);
begin
  var obj := TObjectWithTest.Create(TTest.Create);
  obj.QueueCallSafe(obj.GetTest);
  
  // Free object --> [weak] reference will be cleared (no exception!!)
  obj.Free;
end;

 

[Dalija Prasnikar 1396]

30 minutes ago, Keesver said:

We are looking at safely handling queued calls. The problem is that under some circumstances the object used inside the queued method is freed before the method gets executed. This raises an exception. Can we use this construct to fix it:

Depends what you mean by fixing it?

 

If you are fine that under some circumstances you will get nil inside QueueCallSafe and you check for such situation then you have fixed it. 

Be aware that this code will only work if object is released in the context of main thread - Obj.Free line. If you have that kind of code in some background thread then code in QueueCallSafe will not be safe.

[Anders Melander 1784]

IMO you're bound to get into trouble with that design - it's very fragile.

Firstly you're mixing object and interface references. If you stuck with interfaces, dropped [weak] and used some other mechanism to flag that the object was no longer valid (e.g. a lock protected flag on the object), then it would be fine.

Secondly since you're using [weak] there are limits on what you can safely do with the interface reference. For example supports/queryinterface can't be used on it.

[Keesver 23]

Can you explain why supports/queryinterface can't be used?

[Dalija Prasnikar 1396]

1 hour ago, Keesver said:

Can you explain why supports/queryinterface can't be used?

Because of this https://stackoverflow.com/a/42305385/4267244

 

But, once you have determined your weak reference is valid, you can take strong reference out of it. Once you have strong reference you can do what you need with that reference.

 

procedure TObjectWithTest.QueueCallSafe([weak]AInterface: ITest);
begin
  TThread.ForceQueue(nil, 
    procedure 
    var
      Strong: ITest;
    begin
      if AInterface = nil then
        ShowMessage('nil') 
      else
        begin
          Strong := AInterface;
          ShowMessage('not nil');
        end;
  end);
end;

Again, this only works if there are no background threads involved.