Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lamar 3.0.3 fails in multi threaded scenario #157

Closed
danielpalme opened this issue May 6, 2019 · 16 comments

Comments

@danielpalme
Copy link

commented May 6, 2019

If a class has a dependency on IEnumerable<IFoo> and you register several IFoo implementations, Lamar fails in a multi threaded scenario.
You can get different problems:

  • NULL is passed in the list of IFoos
  • Incorrect number of IFoos is supplied

The problem seems only to occur with version 3.0.3. In previous versions I did not experience the problem.

Here's a sample project to reproduce the issue:
Sample.zip

@jeremydmiller

This comment has been minimized.

Copy link
Contributor

commented May 28, 2019

@danielpalme Sorry this one slipped by me. I'll try to take a look at your sample project in the next couple days

@jeremydmiller

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

Finally looked at this, can reproduce, but out of time. Might be after the 7/4 holiday now. Sorry for the long delay.

@jeremydmiller

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2019

@danielpalme I have no earthly idea why this happens even after a couple hours of fighting with it. The super easy work around though is to just use arguments of type ISimpleAdapter[] or IReadOnlyList<ISimpleAdapter> and it works just fine.

I'm going to leave this open, but the real fix is probably just going to be some documentation.

@CDargis

This comment has been minimized.

Copy link

commented Sep 10, 2019

Bump. I believe we are seeing this issue with integration with MediatR: jbogard/MediatR#438

I'm going to leave this open, but the real fix is probably just going to be some documentation.

Can we please try and dig into this one? I don't want to ask @jbogard to update a framework that is DI container agnostic.

@jbogard

This comment has been minimized.

Copy link

commented Sep 10, 2019

""""""Agnostic""""""

@jeremydmiller

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2019

@CDargis I see absolutely nothing there about Lamar in your stacktraces. I'm gonna wait until it plays out on the MediatR issue.

@CDargis

This comment has been minimized.

Copy link

commented Sep 10, 2019

@jeremydmiller you aren't going to see any Lamar references in MediatR. Consumers can bring their own containers. This problem happens after Lamar has returned a list of instances. One item in this list is null hence the null reference exception seen in the linked issue. Sometimes there are duplicate items.

Does this help move the issue along?
image
image

public interface IClock
{
}

public class Clock : IClock
{
    public void Tick()
    {
    }
}

public class AnotherClock : IClock
{
    public void Tick()
    {
    }
}

class LamarSandbox
{
    public async Task StartAsync()
    {
        Lamar.Container container = new Lamar.Container(x =>
        {
            x.AddTransient<IClock, Clock>();
            x.AddTransient<IClock, AnotherClock>();
        });
        int times = 100000;
        List<Task> tasks = new List<Task>();
        for (int i = 0; i < times; i++)
        {
            Task t = new Task(() => Resolve(container));
            t.Start();
            tasks.Add(t);
        }
        await Task.WhenAll(tasks);
    }

    private void Resolve(Lamar.Container container)
    {
        IEnumerable<IClock> clocks = container.GetInstance<IEnumerable<IClock>>();
        if(clocks.Contains(null))
        {
            Console.Write("null!!");
        }
    }
}
@CDargis

This comment has been minimized.

Copy link

commented Sep 10, 2019

FWIW, I can reproduce in Lamar.Microsoft.DependencyInjection 3.0.0, 3.1.0 and 3.1.2. Can't seem to reproduce in 2.1.0.

@jeremydmiller

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2019

Sigh, okay. MediatR has historically been a huge PITA for me supporting containers and I generally refuse to help support folks who get tangled up in it

Sorry, but I don't have enough bandwidth to fight with this one this week.

@jeremydmiller

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2019

Do see the workarounds above though please

@jbogard

This comment has been minimized.

Copy link

commented Sep 11, 2019

@jeremydmiller

This comment has been minimized.

Copy link
Contributor

commented Sep 11, 2019

@jbogard It's not you man, it's the banana pants things that people do with generics trying to shoehorn crazy middleware strategies into MediatR that kills me

@CDargis

This comment has been minimized.

Copy link

commented Sep 11, 2019

Sorry, but I don't have enough bandwidth to fight with this one this week.

I can definitely empathize with lack of bandwidth.

Do see the workarounds above though please

I can't directly use those workarounds due to the way MediatR consumes containers.

@jbogard It's not you man, it's the banana pants things that people do with generics trying to shoehorn crazy middleware strategies into MediatR that kills me

You do realize this issue has nothing to do with generics or even MediatR? This is just container.GetInstance<IEnumerable<T>> failing in a multi-threaded scenario. I regret mentioning MediatR.

@jeremydmiller

This comment has been minimized.

Copy link
Contributor

commented Sep 11, 2019

@CDargis MediatR in the past has been a huge PITA for me, as I've said a couple times. And it is related to what you're seeing here because of the way Lamar has to go discover the potential open generics/closed generics handlers inside of MediatR at runtime. The internal locking isn't quite right somehow, and that's the root cause of your issue here.

@jorhar

This comment has been minimized.

Copy link

commented Sep 13, 2019

For anyone who stumbles upon this thread experiencing a similar issue....we were unable to come to any type of resolution and unfortunately had to transition back to StructureMap.

As @jeremydmiller described above, there seems to be some issue with how Lamar resolves open generics or collections of open generics under heavy load...or something like that, and MediatR relies on open generic discovery.

At the time of this writing StructureMap has been sunsetted however. MediatR does include a very helpful Container Feature Support page.

@jeremydmiller

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2019

I HAVE A FIX FOR THIS IN A LOCAL BRANCH!!!!!!!!!

This is finally getting taken care of in Lamar v3.1 out in the next 3-4 days.

@jeremydmiller jeremydmiller added this to the 3.1.0 milestone Sep 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.