jbogard / MediatR

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

INotificationHandler constructed, called twice for a single Notification #946

Closed jakenuts closed 11 months ago

jakenuts commented 11 months ago

I'm running into an issue where the existence of a generic notifcation handler constrained to a specific base type is causing a explicit notification handler to be created & called twice when the event it handles is published. Here are the steps to reproduce and a github repository demonstrating the problem.

Demo Repository

-> Handler for EventA is constructed and called twice

-> Handler for generic event type is constructed, called once.

MediatR 12.1.1

jbogard commented 11 months ago

You'll need to verify if this is an issue in the container or MediatR. Check the registrations to see if the handler is registered twice OR if you try to resolve the IEnumerable<INotificationHandler<EventA>> and it has the handlers twice.

jakenuts commented 11 months ago

Thanks, that's what I thought might be happening as I've gotten in the habit of calling AddMediatR in UseShinySevice(this IServiceCollection) extensions and was surprised to see multiple registrations of the same handlers.

Is there a recommended way to allow multiple calls to AddMediatR (say if two separate libraries want to add their handlers and the main app has it's own). I seem to recall (or daydreamed of) some earlier Scrutor based registration checking for duplicates before adding them.

jbogard commented 11 months ago

You can allow multiple calls to AddMediatR but just don't pass in the same assemblies to scan for handlers.

jakenuts commented 11 months ago

Ok, thanks! Here is the debug output when running the demo linked above, as far as I can tell each handler is registered once but the first is instantiated & called twice.

📌 Registered MediatR Services ===

Program: Information: 📰 Publishing TestEventA MediatRTest.Handlers.TestEventAHandler: Information: ✨ TestEventAHandler Created MediatRTest.Handlers.TestEventAHandler: Information: ✨ TestEventAHandler Created MediatRTest.Handlers.TestEventAHandler: Information: ✅ TestEventAHandler Handling MediatRTest.Events.TestEventA MediatRTest.Handlers.TestEventAHandler: Information: ✅ TestEventAHandler Handling MediatRTest.Events.TestEventA Program: Information: 📰 Publishing TestEventB MediatRTest.Handlers.GenericTestEventHandler: Information: ✨ GenericTestEventHandler Created MediatRTest.Handlers.GenericTestEventHandler: Information: ✅ GenericTestEventHandler Handling MediatRTest.Events.TestEventB Program: Information: 🏁 Finished test...

neozhu commented 11 months ago
image

@jakenuts I ran your code, and the test results seem normal. Have you already fixed the issue?

jakenuts commented 11 months ago

Odd! No, no changes and on my system (Win11/VS 17.8 preview 1.0) it looks like this:

image

ibrahimhaouari commented 11 months ago

image looks normal!

jakenuts commented 11 months ago

Hmmm.. Well thanks everyone, I aparently have a "special" build of dotnet.

FWIW I've confirmed that this occurs even if I get the services myself from the service provider, two types registered that can handle the event, two instances returned and they are both the same. So a Microsoft.Extensions.DependencyInjection issue, which thankfully is fixed in the latest 8.0 previews.

Mayron commented 11 months ago

@jakenuts Funny enough, I submitted the same issue (here) last week but must have just missed you closing this! A big coincidence. Turns out this bug was submitted to Microsoft and merged in as a fix to .Net 8 but sadly .Net 8 is in preview and getting companies to migrate over can be tricky or not feasible.

I wish I had seen your issue first but I had to test the problem for a work research task anyway. I doubt I would have known what to look for online until I did the investigation.