It is better to have a synchronized block inside a try block or a try block inside a synchronized block?

Question

For example, is this better?

try {
    synchronized (bean) {
        // Write something              
    }
} catch (InterruptedException e) {
    // Write something
}

Or it's better this:

synchronized (bean) {
    try {           
        // Write something              
    }
    catch (InterruptedException e) {
        // Write something
    }
}

I was wondering which one is best practice. Obviously considering I have to synchronize all the code inside the try block. I'm not talking about the case I've to synchronize only part of the code inside the try (in this case I think it would be better to have the synch block inside the try). My doubts are about the case where I've to synchronize all the try block.

Answer 1

There is no best practice. It only depends if you need the exception handling part inside the synchronized block or not. You might want one or the other, and should choose the one which makes the synchronized block the shortest, while still making the code correct and thread-safe.

Answer 2

It is better to have a synchronized block inside a try block or a try block inside a synchronized block?

Unless you explicitly need the catch to be in the synchronized block, I would make the synchronized section of code as small as possible and have it inside the try/catch. So the first pattern would be better. Then if you do need to do operations in the catch section (like log the exception or re-interrupt the thread, see below), these wouldn't block other threads.

That said, if the synchronized block contains a number of lines (usually not a good idea of course) then I would consider moving the try/catch block closer to the method that throws the exception (prolly wait or notify). With a large number of lines, you run the risk of improperly handling exceptions with large try/catch blocks. Depends a bit on the frame of reference here.

As an aside, make sure that you at least log interrupted exceptions. Never just ignore them. You probably also want to re-interrupt the thread:

try {
   ...
} catch (InterruptedException e) {
   // always a good pattern
   Thread.currentThread().interrupt();
   // handle the interrupt here by logging or returning or ...
}