synchronized on local variable, better than synchronized methods?

Question

I wrote a wrapper around database queries and need to access it from different threads. Therefore my application creates exactly one instance of that helper class and returns it through a getter.

DbConnection dbc = app.getDatabaseConnection();
synchronized (dbc) {
  dbc.doSomething();
}

Is this code safe? As stated here it should work although synchronizing on a local variable. Is this correct as long as the object instance is guaranteed to be the same?

Is it a better approach to make all affected instance methods of DbConnection synchronized?

Answer 1

That's not good design.

Instead of making your DBConnection class inherently thread-safe by making its methods/blocks synchronized when necessary, you force all the clients of this class to explicitely synchronize each time it's needed. So instead of encapsulating the thread-safety in a single, well-dentified class, you distribute this responsibility among all the clients of the class, making the whole thing extremely fragile, and a potential bug extremely hard to find out.

That said, using a single database connection from multiple threads is, in itself, a bad idea.

Answer 2

If all your instance methods of DbConnection need to be synchronized, then make all the methods synchronized. Don't look at the amount of code you write, look only at correctness. If you synchronize each method, you don't have a chance of going back years from now and calling getDatabaseConnection then forgetting to synchronize.