C# Singleton with a Disposable object

Question

Suppose I have a Singleton that loads resources into memory when created, and performs operation on the data when callings its methods. Now suppose, that I want to have the ability to tell the Singleton to release those resources, as I don't expect to be using them in the near future, but also be able to load those resources back in, when the time comes. And I want it all to be thread safe.

What would be the best way to aproach this problem?

Would this example work?:

// Singleton implementation
...

private IDisposable resource;
private bool loadingResources;

private IDisposable Resource {
    get => resource ?? throw new CustomException();
}

// Method A
public void A() {
    var resource = Resource; // Throws CustomException if resource is null
    // Do stuff
}

// Method B
public void B() {
    var resource = Resource;
    // Do stuff
}

public void ReleaseResources() {
    if (resource != null)
        lock (thislock) {
            //resource.Dispose();
            resource = null;
        }
}

public void LoadResources() {
    if (!loadingResources && resource == null)
        lock (thislock)
            if (!loadingResources && resource == null)
            {
                loadingResources = true;
                // Load resources
                resource = CreateResource();
                loadingResources = false;
            }
}

Have you thought about another level of redirection? I.e. If the Singleton Service held a reference to the actual Service, it could redirect all calls to that one. On "ReleaseResources", all you'd have to do is nullify that one reference and GC would take care and ongoing usage could end thair tasks gracefully. All new calls then either fail (because the "secondary" instance ref is null) or lead to recreation of the "secondary" Service. — Fildor, Mar 22 '21 at 07:53
On second thought: Pretty much sounds like some variation of the factory pattern. — Fildor, Mar 22 '21 at 07:55
That's an interesting proposal. Suppose we have a Data class instance inside thesingleton. You would create a reference at the start of each method, i.e. `var data = this.Data;` and then check if it is null. Maybe, just in case, make the Data property thread safe using thread lock too? — Guiorgy, Mar 22 '21 at 09:06
Take a look at this: [Double-checked locking](https://en.wikipedia.org/wiki/Double-checked_locking): *"The pattern [...] can be unsafe. At times, it can be considered an anti-pattern."* This is also relevant: [Double-checked locking in .NET](https://stackoverflow.com/questions/394898/double-checked-locking-in-net) — Theodor Zoulias, Mar 22 '21 at 14:59
@TheodorZoulias, I knew Java had problems with this in the past, but quote from stackoverflow source you gave: "Double-checking locking now works in Java as well as C#", and while I could use `Lazy`, quote from Wiki: "In .NET Framework 4.0, the Lazy class was introduced, **which internally uses double-checked locking** by default". Having said that, if you have a good **recent** article (that stackoverflow post is from 2008) I'd give it a read (PS. I am using Net Core 3.1) — Guiorgy, Mar 22 '21 at 15:27
Guiorgy it's true that the `Lazy.Value` [implementation](https://referencesource.microsoft.com/mscorlib/system/Lazy.cs.html#606a17645b0dc0ff) accesses a non-volatile shared state (the `m_boxed` field) without synchronization, probably because this state can only be mutated once. In your case the `resource` state can be mutated multiple times, and this invalidates the use of the double-checked-locking pattern IMHO. Personally I would avoid going lock-free in any case, knowing that [I am not smart enough](https://stackoverflow.com/a/2529773/11178549) to do it correctly. — Theodor Zoulias, Mar 22 '21 at 15:43

score 1 · Accepted Answer · edited Mar 25 '21 at 04:45

I would suggest separating the resource handling from the actual usage. Assuming the resource requires disposal this could look something like:

    public class DisposableWrapper<T> where T : IDisposable
    {
        private readonly Func<T> resourceFactory;
        private T resource;
        private bool constructed;
        private object lockObj = new object();
        private int currentUsers = 0;

        public DisposableWrapper(Func<T> resourceFactory)
        {
            this.resourceFactory = resourceFactory;
        }

        public O Run<O>(Func<T, O> func)
        {
            lock (lockObj)
            {
                if (!constructed)
                {
                    resource = resourceFactory();
                    constructed = true;
                }
                currentUsers++;
            }

            try
            {
                return func(resource);
            }
            catch
            {
                return default;
            }
            finally
            {
                Interlocked.Decrement(ref currentUsers);
            }
        }

        public void Run(Action<T> action)
        {
            lock (lockObj)
            {
                if (!constructed)
                {
                    resource = resourceFactory();
                    constructed = true;
                }
                currentUsers++;
            }

            try
            {
                action(resource);
            }
            finally
            {
                Interlocked.Decrement(ref currentUsers);
            }
        }

        public bool TryRelease()
        {
            lock (lockObj)
            {
                if (currentUsers == 0 && constructed)
                {
                    constructed = false;
                    resource.Dispose();
                    resource = default;
                    return true;
                }
                return false;
            }
        }
    }

If the resource does not require disposal I would suggest to instead use lazy<T>. Releasing resources would simply mean replacing the existing lazy object with a new one. Letting the old object be cleaned up by the garbage collector.

With the comment of Fildor, I changed the exmple. Do you think that would work? — Guiorgy, Mar 22 '21 at 10:01
@guiorgy No, It is still possible to use the resource after it has been disposed. And you need to duplicate the resource check code in each method. — JonasH, Mar 22 '21 at 10:34
"And you need to duplicate the resource check code in each method". You can readuce it (see edit). If I understand your solution correctly, this is essentially a wrapper for a resource. Any action on the resource happens through the proxy Run. If I wan't to return a result, you would probably just pass a delegate (Func for example) and return the object it returns. The only concern I have, is that it always uses thread locks. Do you think it's possible to decrease that? For one, you don't need to `Interlocked.Increment` as it is already inside a thread lock `lock (lockObj)` — Guiorgy, Mar 22 '21 at 13:28
@Guiorgy yes, this forms a wrapper. If you need to return values you can add a variant that takes a `Func`. Locks are the easiest way to ensure thread safety, and in the vast majority of cases the lock will be released before any waiting thread will transition to the kernel. You could use a publication only thread safety model, but that would allow multiple objects to be constructed, probably not desired if construction is expensive. It is true that `Interlocked.Increment` is not strictly needed, but the impact should be minimal, and being consistent has some value. — JonasH, Mar 22 '21 at 15:15

C# Singleton with a Disposable object

1 Answers1