Search code examples
c#.netmemory-leaksgarbage-collectiondispose

C# .NET preventing an object to dispose what it shouldn't


I work on a big project and a problem occurred: Let's say I have a database loaded to memory, which stores widely-used data. But I must manage if the data is NOT loaded to memory, so I have to download it, then dispose it when I'm done.

But I can make a mistake very easily: I can dispose the database as I had loaded it manually. I want to prevent myself from disposing the database even if I call the Dispose() method on the DB.

I came up with the idea of tracking who can dispose the DB. Of course the only one allowed to do this is the one who created the database instance.

Example of the problem, documented:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace DisposePrevention
{
    /// <summary>
    /// A bottle containing alcoholics
    /// </summary>
    class Bottle:IDisposable
    {
        private static int Id = 0;

        private int localId;

        public Bottle()
        {
            this.localId = Bottle.Id++;
        }

        public void Dispose()
        {
            //do the trick.
        }

        public override string ToString()
        {
            return "Bottle - " + this.localId.ToString();
        }
    }

    /// <summary>
    /// A shelf storing bottles
    /// </summary>
    class Shelf : IDisposable
    {
        public List<Bottle> Content;

        public void Fill()
        {
            if (this.Content == null)
            {
                this.Content = new List<Bottle>();
            }

            for (int i = 0; i < 5; i++)
            {
                this.Content.Add(new Bottle());
            }
        }

        public void Dispose()
        {
            if (this.Content == null)
            {
                return;
            }

            foreach (Bottle b in this.Content)
            {
                b.Dispose();
            }
        }
    }

    /// <summary>
    /// A bartender serving drinks
    /// </summary>
    class Bartender : IDisposable // very simplified.
    {
        public List<Shelf> Shelves;

        public Bartender()
        {
            this.Shelves = new List<Shelf>();

            for (int i = 0; i < 3; i++)
            {
                Shelf s = new Shelf();
                s.Fill();
                this.Shelves.Add(s);
            }
        }

        public void Dispose()
        {
            if (this.Shelves != null)
            {
                foreach (Shelf actualShelf in this.Shelves)
                {
                    if ((actualShelf == null) || actualShelf.Content == null)
                    {
                        continue;
                    }

                    foreach (Bottle bottleItem in actualShelf.Content)
                    {
                        bottleItem.Dispose(); // We can call this, because Content is public, but we shouldn't.
                    }

                    actualShelf.Dispose();
                }

                this.Shelves.Clear();
            }
        }

        /// <summary>
        /// What can we drink, Sir?
        /// </summary>
        public void Print()
        {
            Console.WriteLine("------------------");
            if (this.Shelves != null)
            {
                foreach (Shelf actualShelf in this.Shelves)
                {
                    if ((actualShelf == null) || actualShelf.Content == null)
                    {
                        continue;
                    }

                    foreach (Bottle bottleItem in actualShelf.Content)
                    {
                        Console.WriteLine(bottleItem.ToString());
                    }
                }
            }
            Console.WriteLine("------------------");
        }

        /// <summary>
        /// Two bartenders can use the same source of drinks.
        /// </summary>
        /// <param name="list"></param>
        internal void AttachShelves(List<Shelf> list)
        {
            this.Shelves = list;
        }

        /// <summary>
        /// The boss can fire him, so he no longer gets access to the drinks.
        /// </summary>
        internal void DetachShelves()
        {
            this.Shelves = null;
        }
    }

    class Program
    {
        static void Main(string[] args)
        {
            Bartender john = new Bartender();
            Bartender steven = new Bartender();

            steven.AttachShelves(john.Shelves);

            Console.WriteLine("John:");
            john.Print();
            Console.WriteLine("Steven");
            steven.Print();

            Console.WriteLine("");
            Console.WriteLine("Calling Dispose.");
            Console.WriteLine("");

            john.Dispose(); // we kick John. But at this point, we should've called "john.DetachShelves();"
            Console.WriteLine("John");
            john.Print();

            Console.WriteLine("Steven");
            steven.Print(); // Steven is sad. We should not allow John to dispose the alcoholics.

            Console.ReadLine();
        }
    }
}

Results:

John:
------------------
Bottle - 0
Bottle - 1
Bottle - 2
Bottle - 3
Bottle - 4
Bottle - 5
Bottle - 6
Bottle - 7
Bottle - 8
Bottle - 9
Bottle - 10
Bottle - 11
Bottle - 12
Bottle - 13
Bottle - 14
------------------
Steven
------------------
Bottle - 0
Bottle - 1
Bottle - 2
Bottle - 3
Bottle - 4
Bottle - 5
Bottle - 6
Bottle - 7
Bottle - 8
Bottle - 9
Bottle - 10
Bottle - 11
Bottle - 12
Bottle - 13
Bottle - 14
------------------

Calling Dispose.

John
------------------
------------------
Steven
------------------
------------------
  • I can't use pinned GCHandle-s, to prevent leaks (reference is kept to the object which prevents GC to collect)
  • Generally, I can't count on the Garbage Collector. I must dispose everything I create and GC only collects rarely.
  • The solution with the least modification is the best.
  • I can't use unsafe code... (it's a WPF and Silverlight project)

Idea: I may write a wrapper, but the referencing problem still occurs.

Question:

I want to prevent John from being able to call Dispose() on the Shelves. Is there a "best practice" to do this?

Thanks in advance!

Edit: wrapper

    /// <summary>
    /// A shelf storing bottles
    /// </summary>
    class ShelfWrapped : IDisposable
    {
        public List<Bottle> Content;

        public void Fill()
        {
            if (this.Content == null)
            {
                this.Content = new List<Bottle>();
            }

            for (int i = 0; i < 5; i++)
            {
                this.Content.Add(new Bottle());
            }
        }

        public void Dispose()
        {
            if (this.Content == null)
            {
                return;
            }

            foreach (Bottle b in this.Content)
            {
                b.Dispose();
            }
        }
    }

    /// <summary>
    /// Wrapper for a shelf storing bottles
    /// </summary>
    class Shelf:IDisposable
    {
        private ShelfWrapped InnerShelf = null;

        public Shelf()
        {
            this.InnerShelf = new ShelfWrapped();
        }

        public void Fill()
        {
            if (this.InnerShelf == null)
            {
                this.InnerShelf = new ShelfWrapped();
            }

            this.InnerShelf.Fill();
        }

        public ShelfWrapped GetShelf()
        {
            return this.InnerShelf;
        }

        private List<Bartender> AllowedToDispose = new List<Bartender>();

        public void Dispose(object disposer)
        {
            if (this.AllowedToDispose.Contains(disposer))
            {
                if (InnerShelf != null)
                {
                    this.InnerShelf.Dispose();
                }
            }
        }

        public void Dispose()
        {
            // And again, John can dispose the shelf...
        }
    }

Solution

  • In general, disposable objects should have clear owners, and objects should only dispose things they own. Sometimes it may be necessary to have a field which is owned by some instances of a type but not others; in those cases, one should combine the field with another that indicates whether it owns the instance in question. If a class FunStream inherits from Stream and wraps a Stream, for example, an instance should call Dispose the underlying stream when it is disposed if the code which created it won't be using the underlying stream anymore, but should not dispose it if the code which created it will want to keep using the Stream after the FunStream is disposed. Since the code creating the FunStream will know which pattern it expects, the FunStream constructors or factory methods should offer a parameter to indicate whether the FunStream should assume ownership of the stream.

    The only situation which poses major difficulties occurs when an object is effectively immutable but nonetheless encapsulates resources. Immutable objects are generally freely-shareable and are thus often shared. Although a resource should be released when nobody needs it anymore, predicting which immutable object will be the last one to use a resource is often difficult. The best approach to handling that situation is probably to have a LifetimeManager class which would use some ConditionalWeakTable objects to associate with each class a list of weak references to objects that are still using it, and which would offer a DisposeIfNotNeeded(IDisposable resource, Object owner); which would remove owner from the list of objects that still need the resource and dispose the resource when no more owners remain. I don't know of any existing implementations, however, and the design of such a thing would probably be a bit tricky. Still, using such a class would probably be the cleanest way to ensure properly-timed disposal of shared objects which encapsulate resources.