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

Inconsistent resolution failure #378

Closed
yallie opened this issue Feb 24, 2021 · 9 comments · Fixed by #379
Closed

Inconsistent resolution failure #378

yallie opened this issue Feb 24, 2021 · 9 comments · Fixed by #379
Assignees
Labels
bug Something isn't working
Milestone

Comments

@yallie
Copy link
Contributor

yallie commented Feb 24, 2021

Hi Maksim,

looks like resolution rules are applied a bit differently to decorated and undecorated services:

// the container is set up for the scoped reuse instead of transient by default
using (var scope = container.OpenScope())
{
    var service1 = scope.Resolve<IUndecoratedService>(); // succeeds
    var service2 = scope.Resolve<IDecoratedService>(); // fails
}

Both UndecoratedService and DecoratedService have the same dependencies.
DecoratedService is set up to be created via a factory method like this:

var decoratorSetup = Setup.DecoratorWith(useDecorateeReuse: false);
var factoryMethod = new Func<Lazy<IDecoratedService>, IDecoratedService>(CreateDecoratedService).Method;
c.Register(typeof(IDecoratedService), reuse: Reuse.Transient, made: Made.Of(factoryMethod), setup: decoratorSetup);

// dummy factory method
static IDecoratedService CreateDecoratedService(Lazy<IDecoratedService> lazy)
{
    // pretend that we're returning the imported service decorated with something cool
    return lazy.Value;
}

I believe that both resolutions should yield the same result: either both succeed, or both fail.
But here, the undecorated service succeeds to resolve, and decorated one fails.
Let me prepare the unit test demonstrating this.

@dadhi
Copy link
Owner

dadhi commented Feb 25, 2021

@yallie Thanks for the finding and for the test!

What is the exception in the failing case?

@dadhi
Copy link
Owner

dadhi commented Feb 25, 2021

@yallie

What is the exception in the failing case?

Nevermind, I've got the exception.

@dadhi dadhi self-assigned this Feb 25, 2021
@dadhi dadhi added the bug Something isn't working label Feb 25, 2021
@dadhi dadhi added this to the v4.7.4 milestone Feb 25, 2021
@dadhi dadhi closed this as completed in 0ef9d64 Feb 26, 2021
@dadhi
Copy link
Owner

dadhi commented Feb 26, 2021

@yallie

I have fixed the bug and added a couple of more tests.
Basically, the issue was not dependent on the Decorator usage rather on using the ThrowIfDependencyHasShorterReuseLifespan flag in your setup.

Btw, you may consider the alternative to the Made.Of with the new version of RegisterDelegate which does not rely on the service location and simplifies those kinds of registrations: 0ef9d64#diff-387298765fceb43de567e62ca0605da005595f4519827f979a69665268a479adR95

Here the docs: https://github.com/dadhi/DryIoc/blob/master/docs/DryIoc.Docs/RegisterResolve.md#the-cure---registerdelegate-with-the-dependency-parameters

@dadhi
Copy link
Owner

dadhi commented Feb 26, 2021

@yallie I am planning to release the version with the fix Today.

@dadhi
Copy link
Owner

dadhi commented Feb 26, 2021

DryIoc v4.7.4 is released

@yallie
Copy link
Contributor Author

yallie commented Feb 26, 2021

Maksim, that's great, thanks a lot! 👍

Basically, the issue was not dependent on the Decorator usage rather on
using the ThrowIfDependencyHasShorterReuseLifespan flag in your setup.

Not sure what you mean? I have no such flag in my setup.
Rather, I use WithoutThrowIfDependencyHasShorterReuseLifespan :)

Btw, you may consider the alternative to the Made.Of with the new version of RegisterDelegate

Awesome, thanks for the suggestion! Will look into it.

@yallie
Copy link
Contributor Author

yallie commented Feb 26, 2021

Oh, cool, now the app doesn't start at all :)

Looks like the new rules are more restrictive than before.
The failing class looks something like this:

[Export, PartCreationPolicy.Shared] // it's a singleton that manages the list of sessions
class SessionManager
{
    [Import]
    private ExportFactory<IService> Service { get; set; } // scoped helper service

    public void CreateSession()
    {
       using (var service = Service.CreateExport()) // throws ContainerException
       {
          // DryIoc.ContainerException: 'code: Error.ContainerIsDisposed;'
          // message: Container is disposed and should not be used: 
       }
    }
}

As far as I understand, pre-fix ExportFactory would resolve the service using the current scope.
But now it's bound to the scope that created the SessionManager singleton.
And that scope is already disposed of by the moment ExportFactory is created.

P.S. The class itself remains the same as it was when running on MEF before migrating to DryIoc.
So I think that MEF emulation in DryIoc should support this scenario.
I'll try to create a unit test to reproduce the failure.

@dadhi
Copy link
Owner

dadhi commented Feb 26, 2021

Good :/ more issues to the god of issues.
If you may reproduce with the test will be great.
Untill the fix is prepared u may revert to the v4.7.3

@dadhi dadhi reopened this Feb 26, 2021
@yallie
Copy link
Contributor Author

yallie commented Feb 26, 2021

Perhaps it's the matter of tweaking the MEF's ExportFactory wrapper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants