How to refresh singleton in C#

Question

I have singleton that fetches from DB, hence it is expensive load. It is lazy loaded.

I would like to create a method that refreshes that singleton and populates it once it is required.

the data is DB and very expensive, so I want to refresh it only once in case I have concurrent calls. (that is, if I get 500 calls to refresh, I want to restart the refresh only once)

public static PageData Instance
    {
        get
        {
            if (m_Instance == null)
            {
                lock (instanceLock)
                {
                    if (m_Instance == null)
                    {
                        m_Instance = new PageData();
                    }
                }
            }
            return m_Instance;
        }
    }


public void ReSync()
        {                         
            lock (instanceLock)
            {
                /* Setting to null to force the Instance to re-build */
                m_Instance = null;
                PageData pData = Instance;
            }
        }

thanks

How should the system decide when a refresh is required? Is there a timespan during which all calls should be treated as the same one? — Justin Morgan - On strike, Mar 16 '11 at 13:44

Adam Houldsworth · Accepted Answer · 2011-03-16T13:48:28.767

2

~~This is slightly incorrect, your~~

if (m_Instance == null)

should really be on the inside of the lock.

Sorry, didn't spot that.

Nothing is built in that can make other clients abandon calls silently if you are already refreshing. Timeouts will generate an exception I think. Perhaps maintain a stale DateTime that you can check to avoid doing the refresh on queued callers.

edited Mar 16 '11 at 13:48

answered Mar 16 '11 at 13:39

Adam Houldsworth

63,413
11
150
187

what do you mean? this is double check locking – Himberjack Mar 16 '11 at 13:40
m_Instance == null might be true when you check it, but false once you get to your lock. You need to lock it first to make sure you are the only one acting on the m_Instance. – Adam Houldsworth Mar 16 '11 at 13:41
this is a standard double-check-locking design! – Himberjack Mar 16 '11 at 13:42
Look again -- he checks twice (once outside, then again inside the lock). – Gabe Mar 16 '11 at 13:43
@Adam - he is using double-check locking properly. He checks if it's null both before and after taking the lock. – Andrew Barber Mar 16 '11 at 13:45
Neat! this is the solution I was looking for - Timestamping the callers – Himberjack Mar 16 '11 at 13:49
for those curious about double-check locking: http://csharpindepth.com/Articles/General/Singleton.aspx#dcl – CAD bloke Jul 21 '17 at 02:09

Tengiz · Answer 2 · 2011-03-17T11:17:00.767

In my understanding this should work.

Here is my code:

private static instanceLock = new object();
private static _refreshing = true;

public static PageData Instance
    {
        get
        {
            if (_refreshing)
            {
                lock (instanceLock)
                {
                    if (_refreshing)
                    {
                        m_Instance = new PageData();
                        _refreshing = false; //now allow next refreshes.
                    }
                }
            }
            return m_Instance;
        }
    }


public void ReSync()
        {
            if (!_refreshing)                         
                lock (instanceLock)
                {
                    if (!_refreshing)
                    {
                        _refreshing = true; //don't allow refresh until singleton is called.
                    }
                }
        }

score 1 · Answer 3 · edited May 23 '17 at 12:11

1

Aside from the double-checked locking being broken (correction, apparently it does work but I still find it not particularly pretty), if you have write access to m_Instance why not just set it to new PageData() there and then in ReSync?

edited May 23 '17 at 12:11

Community

1
1

answered Mar 16 '11 at 13:39

Massif

4,327
23
25

I prefer to use the encapsulated property init – Himberjack Mar 16 '11 at 13:41
I've seen this mentioned multiple times, and usually linking to a page such as Massif links here, that complains about Java. This post isn't about Java, though. I've seen no real evidence that double-check locking does not work in .NET. I'd be interested in seeing it, if it exists. – Andrew Barber Mar 16 '11 at 13:44
@Gabe, the implementation of double-checked locking is fine, it's just that the whole pattern is broken, as per the link. (further research finds that it does work now, but it's probably to be avoided: http://stackoverflow.com/questions/394898/double-checked-locking-in-net) – Massif Mar 16 '11 at 13:46
@Gabe: No, this is broken in IA64. The ECMA memory model allows to reorder writes and that may cause the m_Instance to be set before the constructor is fully executed. – mgronber Mar 16 '11 at 14:12
@Gabe: If the `m_Instance` is not volatile, the object reference may be published before the object is fully initialized. This is because the writes may be reordered. Here is some random link about the issue: http://blogs.msdn.com/b/brada/archive/2004/05/12/130935.aspx – mgronber Mar 16 '11 at 14:39
1

@mgronber: So it's not broken, it's just missing a `volatile`. – Gabe Mar 16 '11 at 14:58
1

@Gabe: Well, it is broken if the `volatile` is missing. We do not know the truth, because the example does not have the definition. However, the `volatile` fixes only the initialization of new instance. The code is still broken because the `m_Instance` is read twice. It is first read to check that it is non-null and then it is read again when it is returned. A call to `ReSync()` may set it to null between those two reads. This bug does not even require IA64. – mgronber Mar 16 '11 at 15:13

score 0 · Answer 4 · answered Mar 16 '11 at 13:47

0

How about if you include a "lastRefreshed" member which you also check and lock while refreshing. So if the last refresh occurred within X time, it doesn't happen again?

answered Mar 16 '11 at 13:47

Andrew Barber

39,603
20
94
123

mgronber · Answer 5 · 2011-03-16T14:32:16.483

If you want it to comply with ECMA memory model, I guess the implementation should be something like this (assuming m_Instance is not volatile):

public static PageData Instance
{
    get
    {
        PageData instance = Thread.VolatileRead(ref m_Instance);
        if (instance == null)
        {
            lock (instanceLock)
            {
                instance = Thread.VolatileRead(ref m_Instance);
                if (instance == null)
                {
                    instance = new PageData();
                    Thread.VolatileWrite(ref m_Instance, instance);
                }
            }
        }

        return instance;
    }
}

public void ReSync()
{
    /* Setting to null to force the Instance to re-build */
    Thread.VolatileWrite(ref m_Instance, null);
    PageData pData = Instance;
}

If you have defined m_Instance to be volatile there is only one main difference. The m_Instance needs to be read to local variable before the null check is made, because the ReSync() method may set the shared variable to null. I also removed the lock from ReSync() as it is not really needed. The race to initialize a new instance is safe.

score 0 · Answer 6 · answered Mar 16 '11 at 13:58

As I understand, you expect concurrent calls to Resync method? This really shouldn't happen if you call it only once in 500 requests from clients. Nevertheless, maybe then it's better idea not to just lock on instanceLock, because then singleton would be reinstantiated multiple times anyway, only not at 'same' time.

public void ReSync()
{
  if (!m_IsResyncing)
  {
    lock (m_resyncLock)
    {
      if (!m_IsResyncing)
      {
        m_IsResyncing = true;
        Thread.Sleep(100); // sleep for 100ms to accumulate other locks
        // reinitialize here
        m_IsResyncing = false;
      }
    }
  }
}

score 0 · Answer 7 · answered Mar 17 '11 at 06:37

I may need to understand better how the concurrancy came into this. Your description seems to fit into the scope of System.Cashing namespace. You tell that the object should store its information for an amount of time (some options are available there).

If the next request are same as the previous cashed (you define the criteria of 'identic' yourself) the caller will get the cached copy instead. In real, its means no new db connection. Just a memory request. If the valid cache criteria has passed i.e 30 minutes) the next request are against the database (for another amount of time).

Requires memory but are extremely fast and recommended in scenarios where data are reused.

In terms of refreshing. Each individual cache can be resetted on any choosen time with a .Clear() method — Independent, Mar 17 '11 at 06:44

How to refresh singleton in C#

7 Answers7