Code Junkie Code Junkie - 1 month ago 19
Java Question

Trying to synchronize method within runnable

I have a ConcurrentMap that gets instantiated outside of my runnables, but shared and updated within / accross the runnables. My runnables need to be concurrent, but my concurrentMap update needs to be synchronized to prevent the replacement of previous entrys. Could someone tell me what I'm doing wrong.

public class ExecutionSubmitExample {

public static void main(String[] args) {
//Ten concurrent threads
ExecutorService es = Executors.newFixedThreadPool(10);

List<Future<Example>> tasks = new ArrayList<>();

ConcurrentHashMap<Integer, String> concurrentMap = new ConcurrentHashMap<>();

for (int x = 0; x < 10; x++) {
Example example = new Example(concurrentMap, x);
Future<Example> future = es.submit(example, example);
tasks.add(future);
}

try {
for (Future<Example> future : tasks) {
Example e = future.get();
}

for (Entry<Integer,String> obj : concurrentMap.entrySet()) {
System.out.println("key " + obj.getKey() + " " + obj.getValue());
}
es.shutdown();
} catch (ExecutionException e) {
throw new RuntimeException(e);
} catch (InterruptedException ie) {
throw new RuntimeException(ie);
}
}
}


Runnable

public class Example implements Runnable {

ConcurrentHashMap<Integer, String> concurrentMap;
private int thread;

public Example(ConcurrentHashMap<Integer, String> concurrentMap, int thread) {
this.concurrentMap = concurrentMap;
this.thread = thread;
}

@Override
public void run() {
for (int i = 0; i < 3; i++) {
runAnalysis(i);
}
}

public synchronized void runAnalysis(int index) {
if(concurrentMap.containsKey(index)) {
System.out.println("contains integer " + index);
} else {
System.out.println("put " + index + " thread " + thread);
concurrentMap.put(index, "thread " + thread);
}
}
}


Results - notice index 0 is added multiple times rather than once. It should be added by thread 0 and read as contained by thread 9. I somehow need to lock this method from the other threads until the update has been completed.

put 0 thread 0
put 0 thread 9
put 0 thread 6
put 0 thread 7
put 1 thread 7
put 0 thread 2
put 0 thread 1
put 0 thread 5
put 0 thread 3
put 0 thread 4
contains integer 1
contains integer 1
contains integer 1
contains integer 1
put 2 thread 7
put 1 thread 6
put 1 thread 9
put 1 thread 0
put 0 thread 8
contains integer 2
contains integer 2
contains integer 2
put 2 thread 2
put 2 thread 1
put 2 thread 5
put 2 thread 3
contains integer 1
contains integer 1
contains integer 2
contains integer 2
key 0 thread 8
key 2 thread 3
key 1 thread 0

Answer

synchronized on the method means synchronized on this object. Since you are creating new objects every time

Example example = new Example(concurrentMap, x);

The synchronization is happening on different objects, so nothing blocks.

You need to synchronized on a shared object or use a shared Lock. These can be passed to the Example object or you can use a static field as suggested by Luiggi. In this case, be careful that the field isn't synchronized on anywhere else or it may interfere with this execution.

Comments