jbogard / MediatR

Simple, unambitious mediator implementation in .NET
Apache License 2.0
11.17k stars 1.18k forks source link

HostedService getting re-initialized in every publish #960

Closed gkapellmann closed 1 year ago

gkapellmann commented 1 year ago

Hello,

As the text says, I have a HostedService registered in my .NET 7.0 app:

services.AddHostedService<Worker_Main_Service>();

And I have another script registered as a singleton:

services.AddSingleton<IFolderMonitor, FolderMonitor>();

MediatR is injected in both services and is initialized as:

services.AddSingleton<IFolderMonitor, FolderMonitor>(); services.AddHostedService<Worker_Main_Service>(); services.AddMediatR(cfg => cfg.RegisterServicesFromAssemblies(Assembly.GetExecutingAssembly()));

I can register the publishing of an action from FolderMonitor to Worker_Main_Service. But for some reason, every time that a Publish happens, the hostedservice re-initilized itself. I go through the constructor and the StartAsync functions every single time. Notice that the hosted service is a BackgroundService as it is using the IHostedService interface.

Am I configuring things wrong? Or is this correct behavior from MediatR?

Thank you for your help!

jbogard commented 1 year ago

It's hard to tell without understanding what's going on with your hosted service here. This isn't an issue with MediatR, though. You might try Stack Overflow as well.

gkapellmann commented 1 year ago

Thank you for your answer @jbogard , I also took my time.

I still believe this is a MediatR issue, I have build this very simple project where you can see this behavior. Every time that MediatR is published, the hostedservices with a notification interface gets re-generated, which shouldn't happen.

Furthermore, a non registered hostedservice gets initialized and triggered as well.

Let me know what your thoughts are about this, thank you for your time!

jbogard commented 1 year ago

OK there's something that jumps out right away for me - the hosted services also implement INotificationHandler<>. They should not. That means that the hosted services are registered once with services.AddHostedService and again with services.AddMediatR, each with different service lifetimes.

Create separate notification handlers classes. If there is shared logic needed between multiple classes then pull that out into a common class that can be shared by both, and register that in a container (as transient ideally).

gkapellmann commented 1 year ago

Ok, I see, yes there is a design problem here from my side then.

How would it be the best way to use MediatR between hosted services and/or singletons? Is MediatR meant to be used like this?

So, if data from a Handle has to be used in the hosted service, making a separate class kind of defeats its purpose, no?

jbogard commented 1 year ago

Ohhh yeah, MediatR isn't designed for interprocess communication or whatever this scenario is. I'd throw it out and start over looking for a solution.

gkapellmann commented 1 year ago

OK, so, to settle this issue as fixed, I just wonder, using INotificationHandler<> in a class registers it at a transient automatically? Or it still has to be registered as transient?

Could it be scoped instead?

jbogard commented 1 year ago

I don't like to make handlers anything but transient, and instead prefer to create some other class that is the thing that holds your state. Scoped dependencies need to be disposed and implement IDisposable. Disposable handlers is just icky. They're supposed to be stateless.

gkapellmann commented 1 year ago

Understood.

Thank you for the help!