Thread-safe HashTable

Question

I have a class which contains a HashTable field. How I need to implement get and set methods for exclusive writing to HashTable, but non-exclusive reading?

private Hashtable _data = new Hashtable();

public object this[object key]
{
    get {} // must be non-exclusive 
    set {} // must be exclusive
}

I need my own implementation of getter and setter. Without any additional framework helpers (I asked this question on interview).

In the past I've used a ConcurrentDictionary- not sure that helps. — ek_ny, Mar 31 '13 at 13:05
Have a look at this: http://blogs.msdn.com/b/pfxteam/archive/2010/01/08/9945809.aspx I also think that you are reinventing something that already exists. — jdehaan, Mar 31 '13 at 13:05
Interview question? Are they asking if you know about [`ReaderWriterLockSlim`](http://msdn.microsoft.com/en-us/library/system.threading.readerwriterlockslim.aspx) ? — Nick Butler, Mar 31 '13 at 13:11
@NicholasButler no, I only need to provide my own implementation of getter and setter — WelcomeTo, Mar 31 '13 at 13:12
@jdehaan, may be it already exist, but I need to write my own implementation — WelcomeTo, Mar 31 '13 at 13:13
I can't imagine you could do this "without additional framework helpers." You need *some* kind of synchronization primitive. `Interlocked.CompareExchange`, at minimum. — Jim Mischel, Mar 31 '13 at 13:13
@JimMischel Hmm. but why making `set` method synchronized and `get` method non-synchronized don't help me? — WelcomeTo, Mar 31 '13 at 13:17
@MyTitle ok - then look at the implementation of `ReaderWriterLockSlim` and copy that :) — Nick Butler, Mar 31 '13 at 13:35
The idea is that you want to allow any number of readers concurrently. But if any thread wants to write, it has to get an exclusive lock. So you can have any number of readers, OR you can have one writer. But you can't have readers and writers at the same time. If you allow writing while others are reading, a reader could get incorrect information. — Jim Mischel, Mar 31 '13 at 15:54
@JimMischel thanks, so Chris Gessler's answer to this question isn't correct? — WelcomeTo, Mar 31 '13 at 16:38
That answer is correct in the case of `HashTable`, which is documented to be safe for N readers and one concurrent writer. See the "Thread Safety" discussion in the documentation. Note, however, that this is not true of .NET collection classes in general, which typically follow the rules I outlined above. http://msdn.microsoft.com/en-us/library/system.collections.hashtable.aspx — Jim Mischel, Apr 01 '13 at 11:57

Chris Gessler · Answer 1 · 2013-03-31T13:52:15.040

1

First off explain that you wouldn't use a Hashtable, instead you would use one of the new concurrent collections in .NET 4, like the ConcurrentDictionary which handles all of the synchronization internally.

However, if you still wanted to roll your own synchronized setter, simply add an object to the class and lock on that.

private readonly object _syncRoot = new object();
private Hashtable _data = new Hashtable();

public object this[object key]
{
    get 
    {
        return _data[key];
    }
    set 
    {
        lock(_syncRoot) _data[key] = value;
    } 
}

Also, I forgot that Microsoft included a SyncRoot object in the Hashtable, so this works as well:

private Hashtable _data = new Hashtable();

public object this[object key]
{
    get 
    {
        return _data[key];
    }
    set 
    {
        lock(_data.SyncRoot) _data[key] = value;
    } 
}

edited Mar 31 '13 at 13:52

answered Mar 31 '13 at 13:25

Chris Gessler

22,727
7
57
83

but in this case we can write and read in same time. But I need exclusive writing. – WelcomeTo Mar 31 '13 at 13:34
and why syncRoot is `static`? Thanks – WelcomeTo Mar 31 '13 at 13:36
might not need to be static in this case because the collection is internal to the class. – Chris Gessler Mar 31 '13 at 13:50

score 1 · Answer 2 · answered Mar 31 '13 at 15:07

1

Assuming that locking and waiting on a lock is allowed, then the following logic should work (Note that it uses Java's semantics. I don't know whether it applies to c#):

Object lock
bool write = false
int reads = 0

write(..) {
    synch(lock) {
        while (write || reads > 0) 
            lock.wait();
        write = true;
    }

    ...

    synch(lock) {
        write = false;
        lock.notifyAll();
    }
}

read() {
    synch(lock) {
        while (write) 
            lock.wait();
        reads ++;
    }

    ...

    synch(lock) {
        if (--reads == 0)
            lock.notifyAll();
    }
}

answered Mar 31 '13 at 15:07

Eyal Schneider

22,166
5
47
78

I like your solution, and Java example is better for me :) But shouldn't `lock`, `write`, `reads` variables declared as `volatile`? – WelcomeTo Apr 01 '13 at 04:26
@MyTitle: lock itself should be defined as final (provides thread related guarantees according to the Java Memory Model), and write & reads don't need to be declared as volatile, because all the accesses to them are protected by the same lock. – Eyal Schneider Apr 01 '13 at 05:15

score 0 · Answer 3 · answered Mar 31 '13 at 13:30

0

simply like the following:

get 
{
    return _data[key];
}
set 
{
    lock(_data)
    {
      _data[key] = value;
    }
}

answered Mar 31 '13 at 13:30

Hilmi

3,411
6
27
55

Locking the Hashtable object itself is a terrible idea. – Chris Gessler Mar 31 '13 at 13:31
why? i dont think that i can cause any deadlocks or anything else... – Hilmi Mar 31 '13 at 13:32
1

As long as _data is not exposed this is actually perfectly safe. Hashtable is safe for one writer with concurrent (unsynchronized) readers (in contrast to Dictionary!). – usr Mar 31 '13 at 13:32
I dont see your point sir how can it be insecure? – Hilmi Mar 31 '13 at 13:35
1

It's not "insecure", just bad practice (in this case). But as @usr pointed out, it's safe as long as _data is not exposed. We have 30-40 developers on our team, and who knows what someone will do a few months down the road - but I can be reasonably certain that an object created specifically for synchronization would not be exposed. – Chris Gessler Mar 31 '13 at 14:11

Thread-safe HashTable

3 Answers3

Linked