Singleton factory, sort of....

Question

Sorry if this has been answered elsewhere... I have found a lot of posts on similar things but not the same.

I want to ensure that only one instance of an object exists at a time BUT I don't want that object to be retained past its natural life-cycle, as it might be with the Singleton pattern.

I am writing some code where processing of a list gets triggered (by external code that I have no control over) every minute. Currently I just create a new 'processing' object each time and it gets destroyed when it goes out of scope, as per normal. However, there might be occasions when the processing takes longer than a minute, and so the next trigger will create a second instance of the processing class in a new thread.

Now, I want to have a mechanism whereby only one instance can be around at a time... say, some sort of factory whereby it'll only allow one object at a time. A second call to the factory will return null, instead of a new object, say.

So far my (crappy) solution is to have a Factory type object as a nested class of the processor class:

class XmlJobListProcessor
{
    private static volatile bool instanceExists = false;

    public static class SingletonFactory
    {
        private static object lockObj = new object();

        public static XmlJobListProcessor CreateListProcessor()
        {
            if (!instanceExists)
            {
                lock (lockObj)
                {
                    if (!instanceExists)
                    {
                        instanceExists = true;
                        return new XmlJobListProcessor();
                    }
                    return null;
                }
            }
            return null;
        }
    }

    private XmlJobListProcessor() { }
    ....
    }

I was thinking of writing an explicit destructor for the XmlJobListProcessor class that reset the 'instanceExists' field to false.

I Realise this is a seriously terrible design. The factory should be a class in its own right... it's only nested so that both it and the instance destructors can access the volatile boolean...

Anyone have any better ways to do this? Cheers

Answer 1

just make one and keep it around, don't destroy and create it every minute

"minimize the moving parts"

Answer 2

I would instance the class and keep it around. Certainly I wouldn't use a destructor (if you mean ~myInstance() )...that increases GC time. In addition, if a process takes longer than a minute, what do you do with the data that was suppose to be processed if you just return a null value?

Keep the instance alive, and possibly build a buffer mechanism to continue taking input while the processor class is busy. You can check to see:

if ( isBusy == true )
{
     // add data to bottom of buffer
}
else
{
     // call processing
}

Answer 3

I take everyone's point about not re-instantiating the processor object and BillW's point about a queue, so here is my bastardized mashup solution:

public static class PRManager
{
    private static XmlJobListProcessor instance = new XmlJobListProcessor();
    private static object lockobj = new object();

    public static void ProcessList(SPList list)
    {
         bool acquired = Monitor.TryEnter(lockobj);
            try
            {
                if (acquired)
                {
                    instance.ProcessList(list);
                }
            }
            catch (ArgumentNullException)
            {
            }
            finally
            {
                Monitor.Exit(lockobj);
            }
    }
}

The processor is retained long-term as a static member (here, long term object retention is not a problem since it has no state variables etc.) If a lock has been acquired on lockObj, the request just isn't processed and the calling thread will go on with its business.

Cheers for the feedback guys. Stackoverflow will ensure my internship! ;D

Answer 4

Your original example uses double-check locking, which should be avoided at all costs.

See msdn Singleton implementation on how to do initialize the Singleton properly.

Answer 5

I know .NET 4 is not as widely used, but eventually it will be and you'll have:

private static readonly Lazy<XmlJobListProcessor> _instance =
    new Lazy<XmlJobListProcessor>(() => new XmlJobListProcessor());

Then you have access to it via _instance.Value, which is initialized the first time it's requested.