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

Fixed components resolved from typed factories being disposed along with unrelated objects #439

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Castle Windsor Changelog

## Unreleased

Bugfixes:
- Fixed components resolved from typed factories being disposed along with unrelated objects (@jnm2, #439)

## 4.1.0 (2017-09-28)

Bugfixes:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
// Copyright 2018 Castle Project - http://www.castleproject.org/
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

namespace CastleTests.Facilities.TypedFactory
{
using System;

using Castle.Facilities.TypedFactory;
using Castle.MicroKernel.Registration;

using NUnit.Framework;

public sealed class BurdenAddedToUnrelatedObjectTestCase : AbstractContainerTestCase
{
protected override void AfterContainerCreated()
{
Container.AddFacility<TypedFactoryFacility>();
}

[Test]
public void Object_resolved_from_factory_is_not_added_as_burden_of_object_being_created()
{
Container.Register(
Component.For(typeof(IFactory<>)).AsFactory(),
Component.For<Foo>().LifeStyle.Transient,
Component.For<LongLivedService>().LifeStyle.Singleton,
Component.For<ShortLivedViewModel>().LifeStyle.Transient);

var longLivedService = Container.Resolve<LongLivedService>();
var shortLivedViewModel = Container.Resolve<ShortLivedViewModel>();

var prematureDisposalHandler = new EventHandler((s, e) =>
Assert.Fail("Long-lived service’s connection was disposed when short-lived view model was released."));

longLivedService.SqlConnection.Disposed += prematureDisposalHandler;
Container.Release(shortLivedViewModel);
longLivedService.SqlConnection.Disposed -= prematureDisposalHandler;

Container.Release(longLivedService);
}

public sealed class Foo : IDisposable
{
public event EventHandler Disposed;

public void Dispose() => Disposed?.Invoke(this, EventArgs.Empty);
}

public interface IFactory<T>
{
T Resolve();
void Release(T instance);
}

public sealed class LongLivedService
{
public IFactory<Foo> FooFactory { get; }

public Foo SqlConnection { get; private set; }

public LongLivedService(IFactory<Foo> fooFactory)
{
FooFactory = fooFactory;
}

public void StartSomething()
{
SqlConnection = FooFactory.Resolve();
}

public void Dispose()
{
FooFactory.Release(SqlConnection);
}
}

public sealed class ShortLivedViewModel
{
public IFactory<Foo> FooFactory { get; }
public LongLivedService LongLivedService { get; }

public ShortLivedViewModel(IFactory<Foo> fooFactory, LongLivedService longLivedService)
{
FooFactory = fooFactory;
LongLivedService = longLivedService;
longLivedService.StartSomething();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,14 @@ public virtual object Resolve(IKernelInternal kernel, IReleasePolicy scope)
throw;
}
}
return kernel.Resolve(componentType, additionalArguments, scope);

// Ignore thread-static parent context call stack tracking. Factory-resolved components
// are already tracked by the factory itself and should not be added as burdens just because
// we happen to be resolving in the call stack of some random component’s constructor.

// Specifically, act the same as we would if the timing was slightly different and we were not
// resolving within the call stack of the random component’s constructor.
return kernel.Resolve(componentType, additionalArguments, scope, ignoreParentContext: true);
}

private bool LoadByName(IKernelInternal kernel)
Expand Down
9 changes: 7 additions & 2 deletions src/Castle.Windsor/MicroKernel/DefaultKernel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -702,12 +702,17 @@ protected virtual void RegisterSubSystems()
}

protected object ResolveComponent(IHandler handler, Type service, IDictionary additionalArguments, IReleasePolicy policy)
{
return ResolveComponent(handler, service, additionalArguments, policy, ignoreParentContext: false);
}

private object ResolveComponent(IHandler handler, Type service, IDictionary additionalArguments, IReleasePolicy policy, bool ignoreParentContext)
Copy link
Contributor Author

@jnm2 jnm2 Oct 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original protected method above can be deleted if you don't think users will have inherited from DefaultKernel and called ResolveComponent(IHandler, Type, IDictionary, IReleasePolicy) from inside.

{
Debug.Assert(handler != null, "handler != null");
var parent = currentCreationContext;
var context = CreateCreationContext(handler, service, additionalArguments, parent, policy);
currentCreationContext = context;
var context = CreateCreationContext(handler, service, additionalArguments, ignoreParentContext ? null : parent, policy);

currentCreationContext = context;
try
{
return handler.Resolve(context);
Expand Down
4 changes: 2 additions & 2 deletions src/Castle.Windsor/MicroKernel/DefaultKernel_Resolve.cs
Original file line number Diff line number Diff line change
Expand Up @@ -224,14 +224,14 @@ object IKernelInternal.Resolve(String key, Type service, IDictionary arguments,
return ResolveComponent(handler, service ?? typeof(object), arguments, policy);
}

object IKernelInternal.Resolve(Type service, IDictionary arguments, IReleasePolicy policy)
object IKernelInternal.Resolve(Type service, IDictionary arguments, IReleasePolicy policy, bool ignoreParentContext)
{
var handler = (this as IKernelInternal).LoadHandlerByType(null, service, arguments);
if(handler == null)
{
throw new ComponentNotFoundException(service);
}
return ResolveComponent(handler, service, arguments, policy);
return ResolveComponent(handler, service, arguments, policy, ignoreParentContext);
}

Array IKernelInternal.ResolveAll(Type service, IDictionary arguments, IReleasePolicy policy)
Expand Down
2 changes: 1 addition & 1 deletion src/Castle.Windsor/MicroKernel/IKernelInternal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public interface IKernelInternal : IKernel

IDisposable OptimizeDependencyResolution();

object Resolve(Type service, IDictionary arguments, IReleasePolicy policy);
object Resolve(Type service, IDictionary arguments, IReleasePolicy policy, bool ignoreParentContext = false);
Copy link
Contributor Author

@jnm2 jnm2 Oct 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this parameter is a breaking change to implementors of IKernelInternal. Given the name IKernelInternal, I'm assuming users are expected to not be implementing this interface.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming users are expected to not be implementing this interface.

Correct, it is there because Castle used to have two containers, the MicroKernel and Windsor, the two got merged but obviously still visible from the code base.


/// <summary>
/// Returns a component instance by the key
Expand Down