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.

Lashay

Answer 1

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

Ejaculate

Then if you do need to do operations in the catch section...these wouldn't block other threads. Why would we give up the lock due to an exception? – Ac

Like @Ejaculate said, you have to choose acording to the effect you want, to syncronize the action of the catch block or not, and to have the smallest synchronized block as possible. But, I think you must consider to keep the try/catch block surrounding the code you realy want to guard, avoiding larger blocks, that said, we can summarize these two good programing pratices: 1. Keep small syncronized blocks 2. Keep try/catch blocks surrounding the code you realy want to guard. – Phytobiology

"Why would we give up the lock due to an exception". Because the log calls might be synchronous and are expensive? It depends on the code that is in the catch block @Cratylus. – Ejaculate

@Gray:You mean in case the only thing we can do with an exception is log it.Ok, you are right – Ac

@Craylus, if an exception prevents the code from completing the operation the sync block was designed to do then the lock must be given up. Given that an InterruptedException was used as an example here, it's unclear what this somewhat general exception would mean to an unspecified operation. I arrived at this answer because I wanted to be sure that an exception thrown in a sync block and not caught in the sync block would release the lock in all cases. That's the behavior I need for my code. – Recurrence

Answer 2

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.

Dimity

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.

Isola

Answer 4

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.

Drucilla

Answer 5

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.

Ac

Answer 6

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

Unobtrusive

Answer 7

This is TOO old, but I was reviewing the code of a Junior coleague today and I found a huge bug related to used a Java Lock object like if it was a synchronized block. I was looking for some documentation about Java synchronized not being perfect and just came across this and thought about leaving the solution I have the guy:

In the case of synchronized, it really depends on your logic, since java guarantees the liberation of the lock.

However, when you use Object based locks, like java's ReentrantLock, you must allways do the following:

private final ReentrantLock lock = new ReentrantLock ();
void foo() {
    lock.lock();
    try {
        // Do your things
    } finally {
        lock.unlock();
    }
}

Once you have this structure to ensure lock liberation, you can either encapsulate both the lock and the whole try/finally into another try if you wasnt to handle the catch ourside the lock, or puit it where // Do your things is if you want to handle it from inside.

Also, keep in mind that java locks are not perfect, and like in any programming languaje, the choice of the type of the lock depends on the underlying languaje, how critical the operation is (will someone die if the lock fails, or a controlled software retry once in a million times will do the trick?) and the performance impact of the lock.

If you know nothing about locks, I'd recomend learning the following concepts:

• Monitor (synchronization)
• Mutual exclusion
• Read/write lock pattern
• Read-copy-update
• Semaphore (programming)

Most of the time, what you really need is either a semaphore a multiread singlewrite lock or just a monitor, so knowing the terms will let you search for what you need much faster.

Sawfish