Navigation Menu

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

Scoped call to Resolve() with args seems to leak memory #75

Closed
Leszek-Kowalski opened this issue Feb 18, 2019 · 3 comments
Closed

Scoped call to Resolve() with args seems to leak memory #75

Leszek-Kowalski opened this issue Feb 18, 2019 · 3 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Leszek-Kowalski
Copy link
Contributor

Hi,
It seems that call to Resolve() with args on a scope leads to memory leak with many LightExpressions in root container`s registry. They are not being removed by Dispose() call.

[affects DryIoC.dll 3.1.0-preview-07 and master as well]

        [Test]
        public void Scope_Args_ShouldNotLeakMemory()
        {
            var container = new Container();

            container.Register<ScopedService>(Reuse.InCurrentScope);
            container.RegisterPlaceholder<ScopedServiceConfig>();

            var startMemory = GC.GetTotalMemory(true);

            for (int i = 0; i < 100000; i++)
            {
                using (var scope = container.OpenScope(42))
                {
                    var scopedService = scope.Resolve<ScopedService>(args: new[] { new ScopedServiceConfig() });
                }
            }

            var endMemory = GC.GetTotalMemory(true);
            Assert.Less(endMemory, startMemory * 2);
        }

        class ScopedService
        {
            private readonly ScopedServiceConfig config;

            public ScopedService(ScopedServiceConfig config) => this.config = config;
        }

        class ScopedServiceConfig
        {
        }

Do you want me to look for less hacky means of testing it and to create a PR?

Are there any workarounds ?

@dadhi
Copy link
Owner

dadhi commented Feb 18, 2019

Thanks for posting.

The problem is that ScopedServiceConfig used as key in resolution cache, but you are recreating it each time and it does not implement GetHashCode and Equals.. so no chance for container to reuse the cache.

Add GetHashCode and Equals and re-check, if no memory leak, then it works fine.

@Leszek-Kowalski
Copy link
Contributor Author

Thanks for your reply.
What do you mean by 'used as key' ?
The core purpose of args is to have different transient object (with different hash) every time. I would register it otherwise.

My understanding is args are one time use and should not leave any trace it the registry.

@dadhi
Copy link
Owner

dadhi commented Feb 18, 2019

Make sense, will re-check what's going on there. Now, it will be much faster without cache anyway with interpretation in place.

@dadhi dadhi self-assigned this Feb 19, 2019
@dadhi dadhi added the bug Something isn't working label Feb 19, 2019
@dadhi dadhi added this to the 4.0.0 milestone Feb 19, 2019
dadhi added a commit that referenced this issue Feb 19, 2019
not caching resolved expression with args anymore
optimized RegisterInstance: type checks, late WithResolvedFactory call
optimized Constant expression interpretation
@dadhi dadhi closed this as completed Feb 22, 2019
Leszek-Kowalski pushed a commit to Leszek-Kowalski/DryIoc that referenced this issue Oct 11, 2019
not caching resolved expression with args anymore
optimized RegisterInstance: type checks, late WithResolvedFactory call
optimized Constant expression interpretation
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

No branches or pull requests

2 participants