ThreadDictionary Concurrency Problems

H

harish.seshadri

I wrote a simple thread dictionary and wrote a tester where multiple
threads accessed it and performed operations. Despite having placed
synchronization constructs in the code, I still get exceptions relating
to concurrency. Sometimes I get a ConcurrencyModificationException too.
I just don't get why this is happening since all the synchronization
blocks appear to be in place. What I am missing? Note: I am using Java
1.4.2_07
Below is the code and the log:

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.security.SecureRandom;

public class ThreadDictionary
{
/**
* The thread map
*/
private static Map mThreadMap = new HashMap();

/**
* Get the thread map
* @return The thread map
*/
protected static Map getThreadMap()
{
return mThreadMap;
}

/**
* Make an entry for the key/value pair
* for the particular thread
* @param pThread The thread
* @param pKey The key
* @param pValue The value
*/
public void putEntry(Thread pThread,
String pKey,
Object pValue)
{
Map threadDictionaryMap = null;

// Since we are accessing the thread map
// we should lock access to it, so that
// no other thread can mess with it in the
// meanwhile
synchronized(getThreadMap())
{
// Get the thread dictionary map
threadDictionaryMap = (Map)
mThreadMap.get(pThread.toString());
}

// First we will check to see the thread key
// is already in the map. If it is not there
// then create one and associate it with the
// dictionary
if (null == threadDictionaryMap)
{
// Create a thread dictionary map
threadDictionaryMap = new HashMap();

// Again we are tampering with a map
// accessed by many threads, so synchronize on it
synchronized(getThreadMap())
{
// Associate it with the thread
mThreadMap.put(pThread.toString(),
threadDictionaryMap);
}
}

// Put the entry into the map; this does not require
synchronization
// because this is per thread
threadDictionaryMap.put(pKey, pValue);

synchronized(getThreadMap())
{
System.out.println ("putEntry(Thread,Object,Object): " +
"Thread = " + pThread.toString() +
", Key = " + pKey +
", Value = " + pValue +
", ThreadMap = " + mThreadMap);
}
}

/**
* Get an entry for the given key
* for the particular thread
* @param pThread The thread
* @param pKey The key
* @return The value
*/
public Object getEntry(Thread pThread,
String pKey)
{
synchronized(getThreadMap())
{
System.out.println ("getEntry(Thread,Object): " +
"Thread = " + pThread.toString() +
", Key = " + pKey +
", ThreadMap = " + mThreadMap);
}


Map threadDictionaryMap = null;

// Since we are accessing the thread map
// we should lock access to it, so that
// no other thread can mess with it in the
// meanwhile
synchronized(getThreadMap())
{
// Get the thread dictionary map
threadDictionaryMap = (Map)
mThreadMap.get(pThread.toString());
}

// Does not need to be synchronized
return threadDictionaryMap.get(pKey);
}

/**
* Remove the thread entry from the thread map
* This step is fundamental, otherwise there will be a memory leak
* in the thread map
* @param pThread The thread to lookup by
*/
public void removeThreadEntry(Thread pThread)
{
synchronized(getThreadMap())
{
mThreadMap.remove(pThread.toString());
}
}
}

class ThreadDictionaryTester implements Runnable
{
private static int finishedCount = 0;

public static synchronized int getFinishedCount()
{
return finishedCount;
}

public synchronized void incrementFinishedCount()
{
finishedCount++;
}

// The thread dictionary
private ThreadDictionary mThreadDictionary;

public ThreadDictionaryTester(ThreadDictionary pThreadDictionary)
{
mThreadDictionary = pThreadDictionary;
}

public void run()
{
// The current time key
String key = Long.toString(System.currentTimeMillis());
// The unique value id
String putValue = Long.toString(new SecureRandom().nextLong());

// Put the entry
mThreadDictionary.putEntry(Thread.currentThread(), key,
putValue);

// Get the entry
Object value =
mThreadDictionary.getEntry(Thread.currentThread(), key);

// Compare the values, they must be the same
if (putValue.equals(value))
{
System.out.println(Thread.currentThread() + ": Values are
the same!");
}
else
{
System.out.println(Thread.currentThread() + ": Values are
different!");
}


incrementFinishedCount();
}

public static void main(String [] args) throws Exception
{
int numThreads = 100;
ThreadDictionaryTester threadDictionaryTester = null;
ThreadDictionary threadDictionary = new ThreadDictionary();

for (int i = 0; i < numThreads; i++)
{
threadDictionaryTester = new
ThreadDictionaryTester(threadDictionary);
new Thread(threadDictionaryTester).start();
}

while(getFinishedCount() != numThreads)
{
Thread.sleep(100);
System.out.println("Finished Count: " +
getFinishedCount());
}

System.out.println("Final Finished Count: " +
getFinishedCount());
System.out.println("Done!");
}
}



C:\code\tests>java ThreadDictionaryTester
Finished Count: 0
Finished Count: 0
putEntry(Thread,Object,Object): Thread = Thread[Thread-2,5,main], Key =
11082361
96906, Value = 1780462764071806840, ThreadMap =
{Thread[Thread-2,5,main]={110823
6196906=1780462764071806840}}
getEntry(Thread,Object): Thread = Thread[Thread-2,5,main], Key =
1108236196906,
ThreadMap =
{Thread[Thread-2,5,main]={1108236196906=1780462764071806840}}
Thread[Thread-2,5,main]: Values are the same!
putEntry(Thread,Object,Object): Thread = Thread[Thread-5,5,main], Key =
11082361
96921, Value = -109240809415199841, ThreadMap =
{Thread[Thread-2,5,main]={110823
6196906=1780462764071806840},
Thread[Thread-5,5,main]={1108236196921=-1092408094
15199841}}
getEntry(Thread,Object): Thread = Thread[Thread-5,5,main], Key =
1108236196921,
ThreadMap =
{Thread[Thread-6,5,main]={1108236196906=289687658187770846}, Thread[
Thread-2,5,main]={1108236196906=1780462764071806840},
Thread[Thread-5,5,main]={1
108236196921=-109240809415199841},
Thread[Thread-9,5,main]={1108236196921=-47600
29181353384579}}
Thread[Thread-5,5,main]: Values are the same!
java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextEntry(Unknown
Source)putEntry(Thre
ad,Object,Object): Thread = Thread[Thread-9,5,main], Key =
1108236196921, Value
= -4760029181353384579, ThreadMap =
{Thread[Thread-6,5,main]={1108236196906=2896
87658187770846},
Thread[Thread-2,5,main]={1108236196906=1780462764071806840}, Th
read[Thread-5,5,main]={1108236196921=-109240809415199841},
Thread[Thread-9,5,mai
n]={1108236196921=-4760029181353384579}}

at java.util.HashMap$EntryIterator.next(Unknown Source)
at java.util.AbstractMap.toString(Unknown Source)
at java.lang.String.valueOf(Unknown Source)
at java.lang.StringBuffer.append(Unknown Source)
at
ThreadDictionary.putEntry(ThreadDictionary.java:66)getEntry(Thread,Ob
ject): Thread = Thread[Thread-9,5,main], Key = 1108236196921, ThreadMap
= {Threa
d[Thread-6,5,main]={1108236196906=289687658187770846},
Thread[Thread-2,5,main]={
1108236196906=1780462764071806840},
Thread[Thread-5,5,main]={1108236196921=-1092
40809415199841},
Thread[Thread-9,5,main]={1108236196921=-4760029181353384579}}
at ThreadDictionaryTester.run(ThreadDictionary.java:149)

at java.lang.Thread.run(Unknown Source)
Thread[Thread-9,5,main]: Values are the same!
putEntry(Thread,Object,Object): Thread = Thread[Thread-7,5,main], Key =
11082361
96921, Value = -244273784729546306, ThreadMap =
{Thread[Thread-7,5,main]={110823
6196921=-244273784729546306},
Thread[Thread-6,5,main]={1108236196906=28968765818
7770846}, Thread[Thread-2,5,main]={1108236196906=1780462764071806840},
Thread[Th
read-5,5,main]={1108236196921=-109240809415199841},
Thread[Thread-9,5,main]={110
8236196921=-4760029181353384579}}
getEntry(Thread,Object): Thread = Thread[Thread-7,5,main], Key =
1108236196921,
ThreadMap =
{Thread[Thread-7,5,main]={1108236196921=-244273784729546306}, Thread
[Thread-6,5,main]={1108236196906=289687658187770846},
Thread[Thread-2,5,main]={1
108236196906=1780462764071806840},
Thread[Thread-5,5,main]={1108236196921=-10924
0809415199841},
Thread[Thread-9,5,main]={1108236196921=-4760029181353384579}}
Thread[Thread-7,5,main]: Values are the same!
putEntry(Thread,Object,Object): Thread = Thread[Thread-8,5,main], Key =
11082361
96906, Value = 7948047679099460870, ThreadMap =
{Thread[Thread-7,5,main]={110823
6196921=-244273784729546306},
Thread[Thread-6,5,main]={1108236196906=28968765818
7770846}, Thread[Thread-2,5,main]={1108236196906=1780462764071806840},
Thread[Th
read-5,5,main]={1108236196921=-109240809415199841},
Thread[Thread-8,5,main]={110
8236196906=7948047679099460870},
Thread[Thread-9,5,main]={1108236196921=-4760029
181353384579}}
getEntry(Thread,Object): Thread = Thread[Thread-8,5,main], Key =
1108236196906,
ThreadMap =
{Thread[Thread-7,5,main]={1108236196921=-244273784729546306}, Thread
[Thread-6,5,main]={1108236196906=289687658187770846},
Thread[Thread-2,5,main]={1
108236196906=1780462764071806840},
Thread[Thread-5,5,main]={1108236196921=-10924
0809415199841},
Thread[Thread-1,5,main]={1108236196906=-5722470646593813067}, Th
read[Thread-8,5,main]={1108236196906=7948047679099460870},
Thread[Thread-9,5,mai
n]={1108236196921=-4760029181353384579}}
Thread[Thread-8,5,main]: Values are the same!
putEntry(Thread,Object,Object): Thread = Thread[Thread-1,5,main], Key =
11082361
96906, Value = -5722470646593813067, ThreadMap =
{Thread[Thread-7,5,main]={11082
36196921=-244273784729546306},
Thread[Thread-6,5,main]={1108236196906=2896876581
87770846}, Thread[Thread-2,5,main]={1108236196906=1780462764071806840},
Thread[T
hread-5,5,main]={1108236196921=-109240809415199841},
Thread[Thread-1,5,main]={11
08236196906=-5722470646593813067},
Thread[Thread-8,5,main]={1108236196906=794804
7679099460870},
Thread[Thread-9,5,main]={1108236196921=-4760029181353384579}}
putEntry(Thread,Object,Object): Thread = Thread[Thread-3,5,main], Key =
11082361
96906, Value = 8895510385603133974, ThreadMap =
{Thread[Thread-3,5,main]={110823
6196906=8895510385603133974},
Thread[Thread-7,5,main]={1108236196921=-2442737847
29546306}, Thread[Thread-6,5,main]={1108236196906=289687658187770846},
Thread[Th
read-2,5,main]={1108236196906=1780462764071806840},
Thread[Thread-5,5,main]={110
8236196921=-109240809415199841},
Thread[Thread-1,5,main]={1108236196906=-5722470
646593813067},
Thread[Thread-8,5,main]={1108236196906=7948047679099460870}, Thre
ad[Thread-9,5,main]={1108236196921=-4760029181353384579}}
getEntry(Thread,Object): Thread = Thread[Thread-1,5,main], Key =
1108236196906,
ThreadMap =
{Thread[Thread-3,5,main]={1108236196906=8895510385603133974}, Thread
[Thread-7,5,main]={1108236196921=-244273784729546306},
Thread[Thread-6,5,main]={
1108236196906=289687658187770846},
Thread[Thread-2,5,main]={1108236196906=178046
2764071806840},
Thread[Thread-5,5,main]={1108236196921=-109240809415199841}, Thr
ead[Thread-1,5,main]={1108236196906=-5722470646593813067},
Thread[Thread-8,5,mai
n]={1108236196906=7948047679099460870},
Thread[Thread-9,5,main]={1108236196921=-
4760029181353384579}}
getEntry(Thread,Object): Thread = Thread[Thread-3,5,main], Key =
1108236196906,
ThreadMap =
{Thread[Thread-3,5,main]={1108236196906=8895510385603133974}, Thread
[Thread-7,5,main]={1108236196921=-244273784729546306},
Thread[Thread-6,5,main]={
1108236196906=289687658187770846},
Thread[Thread-2,5,main]={1108236196906=178046
2764071806840},
Thread[Thread-5,5,main]={1108236196921=-109240809415199841}, Thr
ead[Thread-1,5,main]={1108236196906=-5722470646593813067},
Thread[Thread-8,5,mai
n]={1108236196906=7948047679099460870},
Thread[Thread-9,5,main]={1108236196921=-
4760029181353384579}}
Thread[Thread-1,5,main]: Values are the same!
Thread[Thread-3,5,main]: Values are the same!
putEntry(Thread,Object,Object): Thread = Thread[Thread-0,5,main], Key =
11082361
96906, Value = -1998901716773387745, ThreadMap =
{Thread[Thread-0,5,main]={11082
36196906=-1998901716773387745},
Thread[Thread-3,5,main]={1108236196906=889551038
5603133974},
Thread[Thread-7,5,main]={1108236196921=-244273784729546306}, Thread
[Thread-6,5,main]={1108236196906=289687658187770846},
Thread[Thread-2,5,main]={1
108236196906=1780462764071806840},
Thread[Thread-5,5,main]={1108236196921=-10924
0809415199841},
Thread[Thread-1,5,main]={1108236196906=-5722470646593813067}, Th
read[Thread-8,5,main]={1108236196906=7948047679099460870},
Thread[Thread-9,5,mai
n]={1108236196921=-4760029181353384579}}
getEntry(Thread,Object): Thread = Thread[Thread-0,5,main], Key =
1108236196906,
ThreadMap =
{Thread[Thread-0,5,main]={1108236196906=-1998901716773387745}, Threa
d[Thread-3,5,main]={1108236196906=8895510385603133974},
Thread[Thread-7,5,main]=
{1108236196921=-244273784729546306},
Thread[Thread-6,5,main]={1108236196906=2896
87658187770846},
Thread[Thread-2,5,main]={1108236196906=1780462764071806840}, Th
read[Thread-5,5,main]={1108236196921=-109240809415199841},
Thread[Thread-1,5,mai
n]={1108236196906=-5722470646593813067},
Thread[Thread-8,5,main]={1108236196906=
7948047679099460870},
Thread[Thread-9,5,main]={1108236196921=-476002918135338457
9}}
Thread[Thread-0,5,main]: Values are the same!
putEntry(Thread,Object,Object): Thread = Thread[Thread-4,5,main], Key =
11082361
96906, Value = -1903683404827307665, ThreadMap =
{Thread[Thread-0,5,main]={11082
36196906=-1998901716773387745},
Thread[Thread-3,5,main]={1108236196906=889551038
5603133974},
Thread[Thread-4,5,main]={1108236196906=-1903683404827307665}, Threa
d[Thread-7,5,main]={1108236196921=-244273784729546306},
Thread[Thread-6,5,main]=
{1108236196906=289687658187770846},
Thread[Thread-2,5,main]={1108236196906=17804
62764071806840},
Thread[Thread-5,5,main]={1108236196921=-109240809415199841}, Th
read[Thread-1,5,main]={1108236196906=-5722470646593813067},
Thread[Thread-8,5,ma
in]={1108236196906=7948047679099460870},
Thread[Thread-9,5,main]={1108236196921=
-4760029181353384579}}
getEntry(Thread,Object): Thread = Thread[Thread-4,5,main], Key =
1108236196906,
ThreadMap =
{Thread[Thread-0,5,main]={1108236196906=-1998901716773387745}, Threa
d[Thread-3,5,main]={1108236196906=8895510385603133974},
Thread[Thread-4,5,main]=
{1108236196906=-1903683404827307665},
Thread[Thread-7,5,main]={1108236196921=-24
4273784729546306},
Thread[Thread-6,5,main]={1108236196906=289687658187770846}, T
hread[Thread-2,5,main]={1108236196906=1780462764071806840},
Thread[Thread-5,5,ma
in]={1108236196921=-109240809415199841},
Thread[Thread-1,5,main]={1108236196906=
-5722470646593813067},
Thread[Thread-8,5,main]={1108236196906=794804767909946087
0}, Thread[Thread-9,5,main]={1108236196921=-4760029181353384579}}
Thread[Thread-4,5,main]: Values are the same!
Finished Count: 9
Finished Count: 9
Finished Count: 9
Finished Count: 9
Finished Count: 9
 
K

Kevin McMurtrie

I wrote a simple thread dictionary and wrote a tester where multiple
threads accessed it and performed operations. Despite having placed
synchronization constructs in the code, I still get exceptions relating
to concurrency. Sometimes I get a ConcurrencyModificationException too.
I just don't get why this is happening since all the synchronization
blocks appear to be in place. What I am missing? Note: I am using Java
1.4.2_07
Below is the code and the log:
[snip]


This calls mThreadMap.toString(), which calls toString() for each
element, and those elements are active in other threads.

System.out.println ("putEntry(Thread,Object,Object): " +
"Thread = " + pThread.toString() +
", Key = " + pKey +
", Value = " + pValue +
", ThreadMap = " + mThreadMap);




Other threads are modifying the elements here:

// Put the entry into the map; this does not require synchronization
// because this is per thread
threadDictionaryMap.put(pKey, pValue);



Besides that, the code is ugly. Don't use "m_" for statics. Get rid of
getThreadMap(). Don't allow the caller to pass in its own thread
because your code breaks if it's not the current thread. Don't use
Thread.toString() as a hash key because it's an expensive call, it can
be set to a non-unique title, and it's mutable. Use
Thread.currentThread() as the hash key.
 
H

harish.seshadri

Thanks for your suggestions.
I found all of them to be useful except one.
[snip]
This calls mThreadMap.toString(), which calls toString() for each
element, and those elements are active in other threads.
[snip]
This should be rephrased as
This calls mThreadMap.toString(), which calls toString() for each
element, and those elements are active in the current thread.
The elements are only accessible to the current thread. In this example
they are also immutable entries
within the thread dicitionary map. Also I have synchronized over the
thread map when printing it out.
All other threads cannot access it during that time.

Any thoughts?
 
K

Kevin McMurtrie

Thanks for your suggestions.
I found all of them to be useful except one.
[snip]
This calls mThreadMap.toString(), which calls toString() for each
element, and those elements are active in other threads.
[snip]
This should be rephrased as
This calls mThreadMap.toString(), which calls toString() for each
element, and those elements are active in the current thread.
The elements are only accessible to the current thread. In this example
they are also immutable entries
within the thread dicitionary map. Also I have synchronized over the
thread map when printing it out.
All other threads cannot access it during that time.

Any thoughts?

You lost me on that mess of badly quoted text.

Are you asking about mThreadMap.toString()? Take a look at the source
code. For each element in mThreadMap, toString() gets called. It
recurses through the whole tree but you've only synchronized on the
root. Other threads concurrently executing
threadDictionaryMap.put(pKey, pValue), which is not protected by any
synchronization, will cause the Iterator in toString() to fail. Step
through it in a debugger to see it go.

Take out mThreadMap.toString() and it will work. Another fix would be
to hold a synchronization lock on mThreadMap during the entire
putEntry() method.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,905
Latest member
Kristy_Poole

Latest Threads

Top