Java - Why you should avoid method-level synchronization


Using method-level synchronization is considered a bad practice in Java, basically because it is equivalent to synchronized(this). This means the lock you're using (this) is externally exposed. This can lead to some nasty bugs in some situations. Here's a simple example:

class A {
    synchronized void a() {
        new Thread(new MyRunnable(a)).start();
        System.out.println("a()");
        try {
            Thread.sleep(100000000);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
}

class MyRunnable implements Runnable {

    private final A lock;

    MyRunnable(final A lock) {
        this.lock = lock;
    }

    @Override 
    public void run() {
        synchronized (lock) {
            System.out.println("Run");
        }
    }
}

public class Main {
   public static void main(String args[]) throws Exception {
       final A a = new A();
       a.a();
    }
}

If you run it you can see that "Run" is never printed. This is because A#a() is still owning the lock. This is a very simple example that looks it cannot reproduce itself in real-life code... but it can. Finding what it is causing it is going to be a real pain in the ass

So of you use method-level synchronization you should make it really clear in your API documentation, or better, code defensively: create your private lock and synchronize on this lock instead.

Comments

  1. Um, actually the execution never got to starting the other thread, because you are waiting 27.7 hours in a.a(). To confirm this just attach a debugger and check what the main thread's stack is.

    ReplyDelete
    Replies
    1. However if you put
      new Thread(new MyRunnable(this)).start();
      inside `synchronized void a()` it will demonstrate what you're saying, because the new thread will start and be blocked on `synchronized (lock)`. You can see this in debugger too, or put more prints outside the synchronized block.

      Delete
    2. Yes, you're right, that's what I meant to do. Thanks for pointing that out.

      Delete

Post a Comment

Comment, motherf*cker

Popular Posts