How to properly close RegistryKey when chaining them to search for key?

Question

I'm using the following method to search for a key in the registry and return it's value. I pass in a root key to search from and the relative path of the target as a string. As I parse the path, I use the previous key as a base to open the next level. I'ts strictly read-only so, is it necessary to close the base key after I am done with it? And if so, is this the way to do it?

public static string QueryRegistry (RegistryKey root, string path)
{
    return path.Split(Path.DirectorySeparatorChar)
    .Aggregate(root, (r, k) =>
    {
        var key = r?.OpenSubKey(k);
        r?.Close();
        return key;
    }).GetValue(null).ToString();
}

Answer 1

So your basic problem here is that you're creating an indeterminate number of IDisposable objects, and you're not sure when you'll be ready to close them. What I do in a situation like this is create a collection to track them, and then dispose everything in the collection when I'm done. There's some risk here that you won't properly dispose your objects though.

public static string QueryRegistry (RegistryKey root, string path)
{
    List<IDisposable> resourceTracker = new List<IDisposable>() { root };
    string ret = null;
    try 
    {
        ret = path.Split(Path.DirectorySeparatorChar)
        .Aggregate(root, (r, k) =>
        {
            var key = r?.OpenSubKey(k);
            if (key != null) 
            {
                resourceTracker.Add(key);
            }
            return key;
        }).GetValue(null).ToString();
    }
    finally
    {
        foreach (var res in resourceTracker)
        {
            res.Dispose();
        }
    }
    return ret;
}

You could try to do this in a CER, but I'm pretty sure opening the new keys count as allocations which mean the CLR wouldn't treat it as a CER anyway. But this should probably be safe enough.

This could be abstracted into a collection (somewhat like rory.ap suggested) like so:

public class DisposableCollection : IList<IDisposable>, IDisposable
{
    private List<IDisposable> disposables = new List<IDisposable>();

    #region IList<IDisposable> support

    public int Count
    {
        get
        {
            return ((IList<IDisposable>)disposables).Count;
        }
    }

    public bool IsReadOnly
    {
        get
        {
            return ((IList<IDisposable>)disposables).IsReadOnly;
        }
    }

    public int IndexOf(IDisposable item)
    {
        return ((IList<IDisposable>)disposables).IndexOf(item);
    }

    public void Insert(int index, IDisposable item)
    {
        ((IList<IDisposable>)disposables).Insert(index, item);
    }

    public void RemoveAt(int index)
    {
        ((IList<IDisposable>)disposables).RemoveAt(index);
    }

    public void Add(IDisposable item)
    {
        ((IList<IDisposable>)disposables).Add(item);
    }

    public void Clear()
    {
        ((IList<IDisposable>)disposables).Clear();
    }

    public bool Contains(IDisposable item)
    {
        return ((IList<IDisposable>)disposables).Contains(item);
    }

    public void CopyTo(IDisposable[] array, int arrayIndex)
    {
        ((IList<IDisposable>)disposables).CopyTo(array, arrayIndex);
    }

    public bool Remove(IDisposable item)
    {
        return ((IList<IDisposable>)disposables).Remove(item);
    }

    public IEnumerator<IDisposable> GetEnumerator()
    {
        return ((IList<IDisposable>)disposables).GetEnumerator();
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return ((IList<IDisposable>)disposables).GetEnumerator();
    }

    public void AddRange(IEnumerable<IDisposable> range)
    {
        disposables.AddRange(range);
    }

    public IDisposable this[int index]
    {
        get
        {
            return ((IList<IDisposable>)disposables)[index];
        }

        set
        {
            ((IList<IDisposable>)disposables)[index] = value;
        }
    }
    #endregion

    #region IDisposable Support
    private bool disposedValue = false; // To detect redundant calls


    protected virtual void Dispose(bool disposing)
    {
        if (!disposedValue)
        {
            if (disposing)
            {
                foreach(var disposable in disposables)
                {
                    disposable.Dispose();
                }
            }

            // TODO: free unmanaged resources (unmanaged objects) and override a finalizer below.
            // TODO: set large fields to null.

            disposedValue = true;
        }
    }

    // TODO: override a finalizer only if Dispose(bool disposing) above has code to free unmanaged resources.
    // ~DisposableCollection() {
    //   // Do not change this code. Put cleanup code in Dispose(bool disposing) above.
    //   Dispose(false);
    // }

    // This code added to correctly implement the disposable pattern.
    public void Dispose()
    {
        // Do not change this code. Put cleanup code in Dispose(bool disposing) above.
        Dispose(true);
        // TODO: uncomment the following line if the finalizer is overridden above.
        // GC.SuppressFinalize(this);
    }


    #endregion
}

Which could then be used like:

public static string QueryRegistry (RegistryKey root, string path)
{
    string ret = null;
    using (var resourceTracker = new DisposableCollection() { root })
    {
        ret = path.Split(Path.DirectorySeparatorChar)
        .Aggregate(root, (r, k) =>
        {
            var key = r?.OpenSubKey(k);
            if (key != null) 
            {
                resourceTracker.Add(key);
            }
            return key;
        }).GetValue(null).ToString();
    }
    return ret;
}

That class could be extended a bit as well to work in concurrent situations (say using a ConcurrentBag or ConcurrentQueue), and could be extended for other more specific needs (like a collection of RegistryKeys), and would be useful if you find yourself needing this logic in multiple places.