Threading issues with asynchronous messaging to inactive subscription

[dalijap]

There are threading issues in asynchronous messaging with inactive subscription.

Namely code in Post methods checks whether subscription is active before invoking subscription method

TEventBus.Post

for LSubscription in LSubscriptions do begin
  if (LSubscription.Context <> AChannel) or (not LSubscription.Active) then Continue;
  PostToChannel(LSubscription, AMessage, LIsMainThread);

for LSubscription in LSubscriptions do begin
  if not LSubscription.Active then Continue;
  PostToSubscription(LSubscription, AEvent, LIsMainThread);

But, depending on threading mode PostToChannel and PostToSubscription methods can invoke subscription method asynchronously with TThread.Queue or TTask.Run or TThread.CreateAnonymousThread

During that time subscription can get inactive and that can cause crashes by calling subscription method on dead object.

[spinettaro]

Hi @dalijap that is a good point. One additional check can be before the method invoke to check if the Subscriber is not nil:

if not Assigned( ASubscription.Subscriber) then
      Exit;
if not ASubscription.Active then
      Exit;

Other than that we could also think about a callback in case of exception. Because as you can see from the method below the exception will be handled:

  try
    if not ASubscription.Active then
      Exit;

    ASubscription.SubscriberMethod.Method.Invoke(ASubscription.Subscriber, Args);
  except
    on E: Exception do begin
      raise EInvokeSubscriberError.CreateFmt(
        'Error invoking subscriber method. Subscriber class: %s. Event type: %s. Original exception %s: %s.',
        [
          ASubscription.Subscriber.ClassName,
          ASubscription.SubscriberMethod.EventType,
          E.ClassName,
          E.Message
        ]);
    end;
  end;

Then also around the crash, it would be limited to the single thread/task.

We can also think to change the Subscription into an interface.

What do you think? do you have additional suggestions for the issue?

Thanks :)

[spinettaro]

here a PR #65

[dkounal]

Using the last change #65 that was proposed by @spinettaro in a production application that does not have a heavy use of Teventbus, and with TEurekalog enabled to send me messages when application freezes for more than 10sec, I get occasionally a log event like the following:

[spinettaro]

HI @dkounal do you mean there is a freeze now? Do you see a difference with a previous version of DEB (before #65)? Is the freeze time the second column from the right?

[dkounal]

To be honest, it is difficult to compare with before #65, as I had many changes in the code and a bug fix in a component that was used by after Teventmessage comes. Teurekalog has a function where you can request to log when an application UI freezes and have also the callstack. I have put a 10seconds timeframe to log for freezing Their call stack list is not so accurate to say when the app is freezed. But before classes from Eurekalog appear in the above image's stack list, it is a Teventbus in action that exists in the call stack. In the past I had messages like the above, I think more frequently with the previous version of Teventbus. I am not so experienced and I posted if it helps somehow. I will keep following to grab a more helpful message.