C# working with singleton from two different threads

Question

I am using the singleton pattern in a wpf app, but having doubts about how to make it work with multiple threads.

I have a class called Monitor which maintains a list of "settings" to watch, for different "devices". Outline shown below.

On my main thread I am doing Monitor.getMonitor.register(watchlist) or Monitor.getMonitor.unregister(...) depending on the user input and I have a DispatchTimer running every 200ms that does a Monitor.getMonitor.update()

public class Monitor
{
    private Hashtable Master; //key=device, value=list of settings to watch
    private static Monitor instance = new Monitor();
    private Monitor() {}
    public static Monitor getMonitor() 
    {
        return instance;
    }
    public void register(watchlist){...}
    public void unregister(...){...}
    public void update(){...}

}

register()/unregister() perform add/remove to the hastable. update() is only reading stuff out of the hashtable.

Depending on the number of devices and settings, update() is going to be iterating over the hastable and it contents, getting the latest values. The main thread maybe calling register and unregister quite often and I want the gui to stay responsive. Whats a good way to do this?

Do I lock the hashtable, around add/remove and iterate, OR just surrond the iteration part in update with a try catch (ala gracefully fail) to catch any weird state the hashtable might get into(no locking) or is there some better way to do this (if update fails no prob..its going to be running in 200ms again anyway).

Not very sure about what is going on, cause the code as is hasnt really shown any problems which itself is making me a bit uneasy cause it just seems wrong. Thanks for any suggestions...

Answer 1

See my article on singleton implementations to make the singleton fetching itself threadsafe.

Yes, you'll need to lock when you modify or iterate over the hashtable. You could use a ReaderWriterLock (or preferrably ReaderWriterLockSlim in .NET 3.5) to allow multiple readers at a time. If you need to do a lot of work while you're iterating, you could always lock, take a copy, unlock, and then work on the copy - so long as the work doesn't mind the copy being slightly stale.

(If you're using .NET 2.0+, I'd suggest using the generic collections such as Dictionary<TKey, TValue> instead of Hashtable. I'd also suggest you rename your methods in line with .NET conventions. That code's got a distinct Java accent at the moment ;)

Answer 2

Yes, you should lock each operation:

 public class Monitor
 {
     private Hashtable Master; //key=device, value=list of settings to watch
     ...
     private object tableLock = new object();
     public void register(watchlist) 
     {
         lock(tableLock) {
             // do stuff
         }
     }
 }

You shouldn't consider using a try/catch block - exceptions shouldn't be considered as a "normal" situation, and you might end up with a corrupted object state without any exception.

Answer 3

How many rows are there? Unless the update() loop takes a long time to do the iterations, I'd probably lock. If the main thread is potentially doing a lot of register/unregister calls, then update might fail repeatedly -- if it fails for 20 or 30 consecutive calls, is that a problem?

That code looks ok to me. I'd probably make the class sealed. I'd also use a typed dictionary vs. a Hashtable.