It is better to have a synchronized block inside a

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

It has nothing to do with synchronized{try{}} or try{synchronized{}}, but everything to do with synchronized{catch{}} or synchronized{} catch{}. It really depends on what you do in the catch block.

However, taking a guess, for InterruptedException, you usually should do catch{} outside synchronized{}.

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 ...
}

Answer 3

You seem to think this is just an aesthetic question. It isn't. It is a functional question and the answer is dictated by the requirement in each individual case. Each synchronized block should be as large as necessary to contain whatever needs to be synchronized, and no larger.

Answer 4

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 5

Does it matter if your catch block is synchronized? Since you've got "write something" there I'm assuming you're going to be doing some logging which shouldn't need to be synchronized with a good logging framework which would mean the answer is probably no.

In general your goal should be to use as little synchronization as possible. The smaller your synchronization block the less likely you are to run into issues.

Answer 6

In this case, InterruptedException could happen only inside the synchronized block where you could either call wait or sleep or notify.

In general the best practice is to put the try/catch block as close to the code that could throw an exception so that it is easy to identify the code to fix.