If a event handler removes another handler during Event.Invoke, the removed handler is still called

[Ludovic Cléroux]

Let's say there's 2 registered handlers for an event (IEvent<TNotifyEvent>)

TSubscriber1.OnInvoke and TSubscriber2.OnInvoke in this order

1. Event.Invoke(nil) is called
2. FSubscriber1.OnInvoke is called
3. FSubscriber1.OnInvoke calls Event.Remove(FSubscriber2.OnInvoke)
4. FSubscriber2.OnInvoke is still called

Sample :

unit Test;

interface

uses

Classes, Spring, DUnitX.TestFramework;

type

  ISubscriber = interface;

  IEmitter = interface(IInterface)
  ['{49924422-EDA9-4E4F-BFEA-1DC9A676471B}']
    procedure Emit;
    function GetEvent: IEvent<TNotifyEvent>;
    property Event: IEvent<TNotifyEvent> read GetEvent;
  end;

  ISubscriber = interface(IInterface)
  ['{A314302A-4CB7-48BC-9E64-88DA6CBA373B}']
    procedure Attach(AEmitter : IEmitter);
    procedure Detach;
    function GetAttached: Boolean;
    function GetName : string;
    function GetOnEventCalled: Boolean;
    property Attached: Boolean read GetAttached;
    property Name : string read GetName;
    property OnEventCalled: Boolean read GetOnEventCalled;
  end;

  TSubscriber = class(TInterfacedObject, ISubscriber)
  private
    FAttached: Boolean;
    FEmitter : IEmitter;
    FName: string;
    FOnEventCalled: boolean;
    function GetAttached: Boolean;
    function GetName: string;
    function GetOnEventCalled: boolean;
  protected
    procedure OnEvent(Sender : TObject); virtual;
  public
    procedure Attach(AEmitter : IEmitter);
    procedure Detach;
    destructor Destroy; override;
    constructor Create(AName : string);
    property Attached: Boolean read GetAttached;
    property Name: string read GetName;
    property OnEventCalled: boolean read GetOnEventCalled;
  end;

  TRemovingSubscriber = class(TSubscriber)
  private
    FSubscriberToRemove : ISubscriber;
  protected
    procedure OnEvent(Sender : TObject); override;
  public
    constructor Create(AName : string; ASubscriberToRemove : ISubscriber); reintroduce;
  end;

  TEmitter = class(TInterfacedObject, IEmitter)
  private
    FEvent: Event<TNotifyEvent>;
    function GetEvent: IEvent<TNotifyEvent>;
  public
    procedure Emit;
    property Event: IEvent<TNotifyEvent> read GetEvent;
  end;

  [TestFixture]
  TestFixture = class
    published
      [Test]
      procedure Handler_Bug;
  end;

implementation

uses
  System.SysUtils, Winapi.Windows;

procedure TSubscriber.Attach(AEmitter : IEmitter);
begin
  OutputDebugString(PWideChar(Format('Attaching subscriber %s', [FName])));
  FEmitter := AEmitter;
  FEmitter.Event.Add(OnEvent);
  FAttached := True;
end;

constructor TSubscriber.Create(AName: string);
begin
  FName := AName;
end;

destructor TSubscriber.Destroy;
begin
  Detach;
  OutputDebugString(PWideChar(Format('Destroying subscriber %s', [FName])));
  inherited;
end;

procedure TSubscriber.Detach;
begin
  OutputDebugString(PWideChar(Format('Detaching subscriber %s', [FName])));
  if FEmitter = nil then Exit;
  FEmitter.Event.Remove(OnEvent);
  FEmitter := nil;
  FAttached := False;
end;

function TSubscriber.GetAttached: Boolean;
begin
  Result := FAttached;
end;

function TSubscriber.GetName: string;
begin
  Result := FName;
end;

function TSubscriber.GetOnEventCalled: boolean;
begin
  Result := FOnEventCalled;
end;

procedure TSubscriber.OnEvent(Sender: TObject);
begin

  FOnEventCalled := True;

  OutputDebugString(PWideChar(Format('Subscriber %s.OnEvent triggered', [FName])));
end;

{ TEmitter }

procedure TEmitter.Emit;
begin
  FEvent.Invoke(Self);
end;

function TEmitter.GetEvent: IEvent<TNotifyEvent>;
begin
  Result := FEvent;
end;

{ Test }

procedure TestFixture.Handler_Bug;
var
  LEmitter : IEmitter;
  LSub1 : ISubscriber;
  LSub2 : ISubscriber;
begin

  LEmitter := TEmitter.Create;

  LSub1 := TSubscriber.Create('Subscriber 1');
  LSub2 := TRemovingSubscriber.Create('Subscriber 2', LSub1);

  LSub2.Attach(LEmitter);
  LSub1.Attach(LEmitter);

  LEmitter.Emit;

  Assert.IsTrue(LSub2.OnEventCalled);
  Assert.IsFalse(LSub1.OnEventCalled);

  Assert.IsTrue(LSub2.Attached);
  Assert.IsFalse(LSub1.Attached);

end;

{ TRemovingSubscriber }

constructor TRemovingSubscriber.Create(AName: string;
  ASubscriberToRemove: ISubscriber);
begin
  inherited Create(AName);
  FSubscriberToRemove := ASubscriberToRemove;
end;

procedure TRemovingSubscriber.OnEvent(Sender: TObject);
begin
  inherited;
  FSubscriberToRemove.Detach;
end;

end.

[Ludovic Cléroux]

Note that even if the subscriber being removed is destroyed, it will still be called

[Stefan Glienke]

This is as designed and will not be changed.

[Ludovic Cléroux]

In this example, even if LSub2 frees LSub1, LSub1.OnEvent is still called. Is there a builtin way to prevent calling event handlers of freed objects ?

[Stefan Glienke]

No, design your code in a way that it does not happen or use a different approach. There is no way to make this built-in without a huge impact on all the use cases that don't need this.