java multithreading synchronized block not working

Question

I am not able to get why I am getting exception with both : class level lock as well as with object level lock in below code :

It seems object level locking should work here as we are changing and accessing hm(Object) value using different threads, but still we are getting exception(java.util.ConcurrentModificationException).

I tried with all three locking commented in the code.

I know using Hashtable or ConcurrentHashMap we can resolve this problem, but I want to know the concept which is missing using HashMap.

import java.util.HashMap;
class A{
    StringBuilder str = new StringBuilder("ABCD");
    StringBuilder exception = new StringBuilder("");
    HashMap hm = new HashMap();
    public void change() {
        //synchronized(A.class) {
        //synchronized (this){
        synchronized (hm){
            (this.str).append(Thread.currentThread().getName().toString());
            System.out.println(Thread.currentThread().getName()+"::::::"+str);
            hm.put(Thread.currentThread(), Thread.currentThread());
        }
    }
    public void impact() {
        //synchronized(A.class) {
        //synchronized(this) {
        synchronized(hm) {
            System.out.println(Thread.currentThread()+"...Inside impact :::"+hm.get(Thread.currentThread()));
        }
    }
    public void print() {
        System.out.println("Inside print :::"+str);
        System.out.println("Inside print :::exception--"+exception);
    }
}
class B extends Thread{
    A a;
    B(A a){
        this.a=a;
    }
    public void run() {
        try {
            System.out.println("Inside B run::"+a.hm);
            a.change();
            a.impact();
        }
        catch(Exception e){
            StringWriter sw = new StringWriter();
            e.printStackTrace(new PrintWriter(sw));
            System.out.println(sw.toString());
            a.exception.append(sw.toString());
            try {
                sw.close();
            } catch (IOException e1) {
                e1.printStackTrace();
            }
        }
    }
}

class C extends Thread{
    A a;
    C(A a){
        this.a=a;
    }
    public void run() {
    try {
        System.out.println("Inside C run::"+a.hm);
        a.change();
        a.impact();
    }
    catch(Exception e){
        StringWriter sw = new StringWriter();
        e.printStackTrace(new PrintWriter(sw));
        System.out.println(sw.toString());
        a.exception.append(sw.toString());
        try {
            sw.close();
        } catch (IOException e1) {
            e1.printStackTrace();
        }
    }
    }
}
public class multiTest {
    public static void main(String[] args) {
    // TODO Auto-generated method stub
    A a = new A();
    for(int i=0;i<=100;i++) {
        B b = new B(a);
        C c = new C(a);
        b.start();
        c.start();
    }
    try {
        Thread.currentThread().sleep(5000);
    }catch (InterruptedException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }
        a.print();  
    }
}

Answer 1

The problem is on this line:

System.out.println("Inside B run::"+a.hm);

There is sneaky implicit invocation of a.hm.toString() here, and that does sneaky iteration of the map's entries; but you aren't synchronizing on anything, so you don't have exclusive access to the hashmap.

Put it in a synchronized block:

synchronized (a.hm) {
  System.out.println("Inside B run::"+a.hm);
}

(And make hm final; and don't use raw types).