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

Injector.Resolve gets called outside of request context? #49

Closed
huysentruitw opened this issue Jun 27, 2017 · 6 comments
Closed

Injector.Resolve gets called outside of request context? #49

huysentruitw opened this issue Jun 27, 2017 · 6 comments

Comments

@huysentruitw
Copy link
Contributor

huysentruitw commented Jun 27, 2017

There seems to be an issue that sometimes Injector.Resolve gets called outside the WebAPI request context (at least this is how it looks like).

I've created a very simple WebApplication in an attempt to reproducing this issue.

In our application, we're using Autofac for resolving our repositories. Therefore, we created a IDependencyInjector for your library that lets us resolve types from Autofac:

public class Injector : IDependencyInjector
{
    private readonly ILifetimeScope _scope;

    public Injector(ILifetimeScope scope)
    {
        _scope = scope;
    }

    public object Resolve(TypeInfo typeInfo)
    {
        var innerScope = _scope.BeginLifetimeScope();
        return innerScope.Resolve(typeInfo);
    }
}

The complete source of this test web application can be found here

Now, sometimes, during a request, an ObjectDisposedException is thrown by Autofac because it seems like _scope is already disposed, which is the AutofacWebRequest scope as we register it as InstancePerRequest() like this: builder.RegisterType<Injector>().As<IDependencyInjector>().InstancePerRequest();.

image

This can only be the case after the WebAPI request is completed. This makes me think that we're getting into the Resolve method too late. This only happens on rare occasions but we see it happening every day in our production code with only 1 active user.

I have written a small Console application that sends out graphql queries in a loop from 10 simultaneous threads and I always get the exception somewhere between request 200 and 1500.

I've also compared the complete stacktrace of a successful Resolve with the one from a failing Resolve and they both look the same (apart from the method inside Autofac that throws the exception).

Stacktrace of failing Resolve can be found here, the successful Resolve can be found here.

The console application for firing requests looks like this:

class Program
{
    const string json = "{\"query\":\"query {\n cars {\n brand\n wheels {\n size\n    }\n  }\n}\",\"variables\":null}";

    static ManualResetEvent stop = new ManualResetEvent(false);

    static HttpClient Client = new HttpClient();
    static int success = 0, failure = 0;

    static void Main(string[] args)
    {
        Client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));

        for (int i = 0; i < 10; i++) new Thread(new ThreadStart(DoIt)).Start();

        while (!Console.KeyAvailable)
        {
            Console.WriteLine("success: {0}  failure: {1}", success, failure);
            Thread.Sleep(500);
        }

        stop.Set();
    }

    static void DoIt()
    {
        while (!stop.WaitOne(1, false))
        {
            var content = new StringContent(json, Encoding.UTF8, "application/json");
            var response = Client.PostAsync("http://localhost:57712/api/graph", content).Result;
            if (response.IsSuccessStatusCode) ++success; else ++failure;
        }
    }
}

We could probably register everything as InstancePerDependency to decouple it from any lifetimescope, but I consider this as a workaround rather than a fix. I really want to track down this one, but I'm out of inspiration. Do you have any idea what can go wrong?

@huysentruitw
Copy link
Contributor Author

huysentruitw commented Jun 27, 2017

One possibility would be the usage of ConfigureAwait(false) in the Conventions library that causes loosing the web request context see https://stackoverflow.com/a/40220190/1300910 Removed all ConfigureAwait(false) calls and it still fails.

Remark: we're using normal ASP.NET WebAPI 2, not ASP.NET Core.

@huysentruitw
Copy link
Contributor Author

Found the root of the problem. There seems to be only one dependency injector and it is kept on the engine (see https://github.com/graphql-dotnet/conventions/blob/master/src/GraphQL.Conventions/Adapters/Engine/GraphQLExecutor.cs#L87) although the method WithDependencyInjector is a member of the GraphQLExecutor/

This means, that in a multi-threading environment thread 1 can override the dependency injector set by thread 2, which is exactly what happens when we try to process multiple requests at the same time. I consider this as a bug, but I already have a fix which I will send as a PR in a moment. I only need to add a unit-test to cover this situation.

huysentruitw pushed a commit to CogniStreamer/conventions that referenced this issue Jun 29, 2017
@tlil
Copy link
Collaborator

tlil commented Jun 29, 2017

Ah sorry dude. Just saw this now. Thanks for looking into it - nice catch! Will look at your PR here later today and get that merged. Thanks!

@huysentruitw
Copy link
Contributor Author

Thank you. I'm leaving on a holiday after today, so if there are any issues with the PR, I can fix them today, otherwise it might take some time.

In the meantime, we're creating an engine for each request 😮, not performant, but we have to prepare for a demo 😄

tlil added a commit that referenced this issue Jun 30, 2017
Give each executor its own IDependencyInjector #49
@tlil
Copy link
Collaborator

tlil commented Jun 30, 2017

👍 Haha, yeah. I've merged it now - looks good! Version 1.2.9 is indexing on NuGet as we speak :-)

Thanks for fixing this!

@tlil tlil closed this as completed Jun 30, 2017
@tlil
Copy link
Collaborator

tlil commented Jun 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants