2

We're using a library that uses pooled objects (ServiceStack.Redis's PooledRedisClientManager). Objects are created and reused for multiple web requests. However, Dispose should be called after each use to release the object back into the pool.

By default, Ninject only deactivates an object reference if it has not been deactivated before.

What happens is that the pool instantiates an object and marks it as active. Ninject then runs the activation pipeline. At the end of the request (a web request), Ninject runs the deactivation pipeline which calls Dispose (and thus the pool marks the object as inactive). The next request: the first pooled instance is used and the pool marks it as active. However, at the end of the request, Ninject does not run its deactivation pipeline because the ActivationCache has already marked this instance as deactivated (this is in the Pipeline).

Here's a simple sample that we've added in a new MVC project to demonstrate this problem:

public interface IFooFactory
{
    IFooClient GetClient();
    void DisposeClient(FooClient client);
}

public class PooledFooClientFactory : IFooFactory
{
    private readonly List<FooClient> pool = new List<FooClient>();

    public IFooClient GetClient()
    {
        lock (pool)
        {
            var client = pool.SingleOrDefault(c => !c.Active);

            if (client == null)
            {
                client = new FooClient(pool.Count + 1);
                client.Factory = this;
                pool.Add(client);
            }

            client.Active = true;

            return client;
        }
    }

    public void DisposeClient(FooClient client)
    {
        client.Active = false;
    }
}

public interface IFooClient
{
    void Use();
}

public class FooClient : IFooClient, IDisposable
{
    internal IFooFactory Factory { get; set; }
    internal bool Active { get; set; }
    internal int Id { get; private set; }

    public FooClient(int id)
    {
        this.Id = id;
    }

    public void Dispose()
    {
        if (Factory != null)
        {
            Factory.DisposeClient(this);
        }
    }

    public void Use()
    {
        Console.WriteLine("Using...");
    }
}

public class HomeController : Controller
{
    private IFooClient foo;

    public HomeController(IFooClient foo)
    {
        this.foo = foo;
    }

    public ActionResult Index()
    {
        foo.Use();
        return View();
    }

    public ActionResult About()
    {
        return View();
    }
}

// In the Ninject configuration (NinjectWebCommon.cs)
private static void RegisterServices(IKernel kernel)
{
    kernel.Bind<IFooFactory>()
        .To<PooledFooClientFactory>()
        .InSingletonScope();

    kernel.Bind<IFooClient>()
        .ToMethod(ctx => ctx.Kernel.Get<IFooFactory>().GetClient())
        .InRequestScope();
}

The solutions that we've come up with thus far are:

  1. Mark these objects as InTransientScope() and use other deactivation mechanism (like an MVC ActionFilter to dispose of the object after each request). We'd lose the benefits of Ninject's deactivation process and require an indirect approach to disposing of the object.

  2. Write a custom IActivationCache that checks the pool to see if the object is active. Here's what I've written so far, but I'd like some one else's eyes to see how robust it is:

    public class PooledFooClientActivationCache : DisposableObject, IActivationCache, INinjectComponent, IDisposable, IPruneable
    {
        private readonly ActivationCache realCache;
    
        public PooledFooClientActivationCache(ICachePruner cachePruner)
        {
            realCache = new ActivationCache(cachePruner);
        }
    
        public void AddActivatedInstance(object instance)
        {
            realCache.AddActivatedInstance(instance);
        }
    
        public void AddDeactivatedInstance(object instance)
        {
            realCache.AddDeactivatedInstance(instance);
        }
    
        public void Clear()
        {
            realCache.Clear();
        }
    
        public bool IsActivated(object instance)
        {
            lock (realCache)
            {
                var fooClient = instance as FooClient;
                if (fooClient != null) return fooClient.Active;
    
                return realCache.IsActivated(instance);
            }
        }
    
        public bool IsDeactivated(object instance)
        {
            lock (realCache)
            {
                var fooClient = instance as FooClient;
                if (fooClient != null) return !fooClient.Active;
    
                return realCache.IsDeactivated(instance);
            }
        }
    
        public Ninject.INinjectSettings Settings
        {
            get
            {
                return realCache.Settings;
            }
            set
            {
                realCache.Settings = value;
            }
        }
    
        public void Prune()
        {
            realCache.Prune();
        }
    }
    
    
    
    // Wire it up:
    kernel.Components.RemoveAll<IActivationCache>();
    kernel.Components.Add<IActivationCache, PooledFooClientActivationCache>();
    
  3. Specifically for ServiceStack.Redis's: use the PooledRedisClientManager.DisposablePooledClient<RedisClient> wrapper so we always get a new object instance. Then let the client object become transient since the wrapper takes care of disposing it. This approach does not tackle the broader concept of pooled objects with Ninject and only fixes it for ServiceStack.Redis.

    var clientManager = new PooledRedisClientManager();
    
    kernel.Bind<PooledRedisClientManager.DisposablePooledClient<RedisClient>>()
        .ToMethod(ctx => clientManager.GetDisposableClient<RedisClient>())
        .InRequestScope();
    
    kernel.Bind<IRedisClient>()
        .ToMethod(ctx => ctx.Kernel.Get<PooledRedisClientManager.DisposablePooledClient<RedisClient>>().Client)
        .InTransientScope();
    

Is one of these approaches more appropriate than the other?

1 Answer 1

1

I have not use Redis so far so I can not tell you how to do it correctly. But I can give you some input in general:

Disposing is not the only thing that is done by the ActivationPipeline. (E.g. it also does property/method injection and excuting activation/deactivation actions.) By using a custom activation cache that returns false even though it has been activated before will cause that these other actions are executed again (E.g. resulting in property injection done again.)

1
  • Hi Remo - in the context of the sample that I created above withe the IFooFactory, is there a way to re-use pooled objects as if they're a new reference? Jun 12, 2013 at 19:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.