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

Issue with IServiceProvider, Scope, Sub scope #5312

Open
MihaiStrejer opened this issue Nov 29, 2019 · 12 comments
Open

Issue with IServiceProvider, Scope, Sub scope #5312

MihaiStrejer opened this issue Nov 29, 2019 · 12 comments
Labels

Comments

@MihaiStrejer
Copy link

MihaiStrejer commented Nov 29, 2019

I've been waiting for #3399 to be resolve but when I ran my test I've got a strange result. It has to do with creating a sub scope using the function scoped IServiceProvider. In other IOC frameworks an instance that got created by the scope and tries to resolve IServiceProvider would get the creator scope reference. This is not the case in azure functions IOC.
More so between subsequent runs the HashCode of the function scope does not change however some cleanup appears to be done since the scoped services are not the same.

Possibly related to #4914

I have a created sample repository here: Repo
The code is fairly simple I'm going to list some details below:

Objects lifetime:
TestService, MedRequestHandler - transient
MediatR - scoped (as well as the ServiceFactory)
MyStateService - scoped

Azure Function Scope > (DryIoc and the .Net container create new scopes in the FunctionEntry)
     FunctionEntry (Function1) >
         CreateSubScope and load a state >
             Resolve TestService that checks IServiceProvider >
                 MediatR create and run request >
                     MedRequestHandler that checks IServiceProvider

Normally you would expect to see the scope and state created in CreateSubScope to propagate down the line.

Image result

And some final questions:

  1. Up until now I've ran with a custom side container that behaves as expected (similar to the example DryIocSideContainer). Are there any potential issues with running a side container in functions, at least until the scope issue is fixed?

  2. Does the scope created by azure functions handle IDisposables? Does it "behave" similarly to the default DryIoc container?

@t-l-k
Copy link

t-l-k commented Nov 30, 2019

(1) I have read in other tickets here that others are using "side containers", I think @APIWT mentioned he was using this approach to counter DI issues (can't find link).

In relation to your question (2), I have witnessed that the DryIOC container can handle and does handle disposables.

The Functions runtime does however appear to execute the dispose in a different logical execution context. As a practical example, any AsyncLocal statics you may have captured during the execution of your function will no longer be in scope when Dispose is called, but this shouldn't be any major problem (e.g. just use instance fields).

I too have noticed, as have you, that the IServiceProvider is effectively a singleton. It appears to rely on a resolver abstraction internally which obtains an IScope via the logical execution context. This is currently implemented, again, as an AsyncLocal (integrated in this commit). So the IServiceProvider singleton ultimately delegates to the IScope which I believe should contain the dependencies scoped to the Function's active logical execution context. This was the fix for #3399

Theoretically this seems sound, so it being a singleton doesn't seem too offensive (but perhaps I've just rationalised this perspective too much?). In practice at present, there is a bug in there somewhere (my #4914 and @heikkilamarko #5098), or its not quite implemented right - the timing of the capture of the IScope, needs to be just right in relation to the logical execution context.

@fabiocav fabiocav added this to the Triaged milestone Dec 1, 2019
@fabiocav
Copy link
Member

fabiocav commented Dec 1, 2019

Thanks for reporting this. I'll be blocking some time to look at this area soon, and have identified an issue impacting the scenario reported by @t-l-k, just pending some validation before I can get that in.

@hgrafael2010
Copy link

Hello, any progress so far?

@glitch100
Copy link

Any progress on this?

@jeffhollan jeffhollan added the P1 label Feb 19, 2020
@micklaw
Copy link

micklaw commented Sep 17, 2020

Possibly flogging a dead horse here, but I am still seeing this issue, was there ever any solution to this?

It really does seem like Scoped dependencies are not always honoured through our request lifecycle.

@jeffhollan, this was marked as a P1 in Feb, it would be great if this was acknowledged or even closed with a description regarding best practice to work around.

Cheers

@dadhi
Copy link

dadhi commented Sep 30, 2020

Hello, I am author of DryIoc.

I have read the issue but still not sure that I get the problem. There is a lot of references and split info.

I am willing to help if someone explains what DryIoc version and the rules are used.. What is expected behavior and what is the problem.

@micklaw
Copy link

micklaw commented Sep 30, 2020

Hey @dadhi

Thanks for taking the time to respond here.

From what I have seen I don’t think this is necessarily an issue with DryIoC, but instead how the container and it’s child scopes are being managed within the function runtime.

As the example repo above shows, a static container, both DryIoC and a new IServiceCollection, both resolve dependencies as expected.

The issue appears when you try and resolve a service from the primary Functions IOC container which is using DryIoC under the hood. In these instances the lifetimes of the IServiceProvider injected to our services are not as expected. In the repo above in all occasions it resolves a scoped service from what appears to be the wrong scope.

I am not sure if this has something to do with the AsyncLocal where the scope is stored, or from how the Azure function starts up (I was seeing this on a cold start using Mediatr behaviours), warm start was fine.

It I am sure is a difficult problem to diagnose, I had a quick scan but ended in deep without the time.

We worked around it be simply removing our scoped dependencies, but this is obviously not ideal.

Any help would be appreciated. Thanks again.

@dadhi
Copy link

dadhi commented Sep 30, 2020

DryIoc scoping behavior depends on the rules. By default it does not use the shared scope context - the scopes tree is bound to the container instance. But you opt-in it with the shared scope context where the the scopes are tracked. One of the predefined contexts is the AsyncExecutionFlowScopeContext where scopes bound to the async call context (basically to the AsyncLocal). That's why it is important for me to know how container(s) is configured and the details of the use-case.

https://github.com/dadhi/DryIoc/blob/master/docs/DryIoc.Docs/ReuseAndScopes.md#scopecontext

@micklaw
Copy link

micklaw commented Sep 30, 2020

the scopes tree is bound to the container instance. But you opt-in it with the shared scope context where the the scopes are tracked ... That's why it is important for me to know how container(s) is configured and the details of the use-case.

The example created by @MihaiStrejer above reproduces this 100% of the time from what I could see. As I say, the complexities of the function runtime and also DryIoC make this problem, for me anyway, non trivial to resolve and in honesty feel it requires buy-in by the MS team to resolve it. I am not sure it something mere mortal like myself would be best place to resolve in any sort of timely manner, plus we do pay for Azure functions so don't think it unfair we ask the question.

Again, I am really appreciatove of the work happening here, but from the MS side I feel this should be a higher priority than it currently is. I'm not really sure this should be your resposibility @dadhi, but I do appreciate you looking.

@micklaw
Copy link

micklaw commented Sep 30, 2020

DryIoc scoping behavior depends on the rules ...
https://github.com/dadhi/DryIoc/blob/master/docs/DryIoc.Docs/ReuseAndScopes.md#scopecontext

Kuds on those docs too @dadhi, some great reading here and food for thought for my problem we worked around. I'll be sure to give it a go 👍

@MihaiStrejer
Copy link
Author

MihaiStrejer commented Oct 5, 2020

That's why it is important for me to know how container(s) is configured and the details of the use-case.

Indeed, and as @micklaw stated this is not a problem with DryIoc, but a problem with the azure function host.
As of the time I've looked into it (the time the issue was created, early version of v2 host) I've seen 2 problems that contribute to this "issue":

  1. The elephant in the room was the fact that the azure function host buildup sequence is locked, there is no easy way to provide a custom DI container. Thankfully they've chosen to use DryIoc, which is in my opinion one of the better libraries, if not the best. More so, because the instance is locked into the host startup, certain services like the Configuration, Logging and others are created in the implicit container and can't be customized. This creates an additional level of complexity when we want to run another "side container". Again nothing that is a show stopper, just more complexity in "copy/create" the instances you need over to the new custom container. Not ideal but workable and I've seen hints to an initiative to open up more of the startup sequence to customization.

  2. In addition to that the azure host unfortunately uses a proprietary/customized version of DryIoc (copied over and not referenced via nuget). I assume that was done to quickly accommodate various scenarios and expediate the release of the host. I'm going off on a limb here but its because of this reason we now have the scope bug(s).

I think this is how the container is setup:

@t-l-k
Copy link

t-l-k commented Oct 6, 2020

@MihaiStrejer FYI It is possible that the fix is in for this issue via ticket #5098

@fabiocav fabiocav removed this from the Triaged milestone Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants