views:

13468

answers:

6

If I have a util class with static methods that will call hibernate functions to accomplish basic data access. I am wondering if making the method synchronized is the right approach to ensure thread-safety.

I want this to prevent access of info to the same DB instance. However, I'm now sure if the following code are preventing getObjectById being called for all Classes when it is called by a particular class.

public class Utils {
     public static synchronized Object getObjectById (Class objclass, Long id) {
           // call hibernate class
         Session session = new Configuration().configure().buildSessionFactory().openSession();
         Object obj = session.load(objclass, id);
         session.close();
         return obj;
     }

     // other static methods
}
+7  A: 

Static methods use the class as the object for locking, which is Utils.class for your example. So yes, it is OK.

starblue
+2  A: 

If it is something to do with the data in your database, why not utilize database isolation locking to achieve?

codemeit
I don't have any database background. Now I know!! Thanks for pointing it out! :)
tomato
A: 

To answer your question, yes it does: your synchronized method cannot be executed by more than one thread at a time.

David Zaslavsky
+3  A: 

Why do you want to enforce that only a single thread can access the DB at any one time?

It is the job of the database driver to implement any necessary locking, assuming a Connection is only used by one thread at a time!

Most likely, your database is perfectly capable of handling multiple, parallel access

oxbow_lakes
I am betting it is/was a workaround for some transactional issue. I.e., the solution doesn't solve the true problem
matt b
I didn't know that....I thought I'd have to manually implement this. Thanks for pointing it out! :)
tomato
+8  A: 

By using synchronized on a static method lock you will synchronize the whole class

So your assumption is correct.

I am wondering if making the method synchronized is the right approach to ensure thread-safety.

Not really. You should let that work to your RDBMS instead. They are good at this kind of stuff.

The only thing you will get by synchronizing the access to the database is to make your application terribly slow. Further more, in the code you posted you're building a Session Factory each time, that way, your application will spend more time accessing the DB than performing the actual job.

Imagine the following scenario:

Client A and B attempt to insert different information into record X of table T.

With your approach the only thing you're getting is to make sure one is called after the other, when this would happen anyway in the DB, because the RDBMS will prevent them from inserting half information from A and half from B at the same time. The result will be the same but only 5 times ( or more ) slower.

Probably it could be better to take a look at the "Transactions and Concurrency" chapter in the Hibernate documentation. Most of the times the problems you're trying to solve, have been solved already and a much better way.

OscarRyz
Very helpful answer!THANKS! So Hibernate takes care of cnocurrency by "optimistic locking". Then there is no need to use "synchronized" methods at all for resolve any data-access concurrecy?? Use "synchronized" methods only if the data are not stored in the database?? ..when DO you use them??
tomato
1) I think there are some means to use pessimistic locking too. 2) Nope, the RDBMS can do that work. 3) If the data is accessed by multiple threads at the same time. 4) synchronization is useful when two thread have to share data. If they don't need to, then much better!
OscarRyz
Any fast food restaurant uses multithread. One thread takes you order and use another thread to prepare it, and continues with the next customer. The synchronization point works only when they interchange information to know what to prepare. Following a model like that really simplifies life.
OscarRyz
+4  A: 

To address the question more generally...

Keep in mind that using synchronized on methods is really just shorthand (assume class is SomeClass):

synchronized static void foo() {
    ...
}

is the same as

static void foo() {
    synchronized(SomeClass.class) {
        ...
    }
}

and

synchronized void foo() {
    ...
}

is the same as

void foo() {
    synchronized(this) {
        ...
    }
}

You can use any object as the lock. If you want to lock subsets of static methods, you can

class SomeClass {
    private static final Object LOCK_1 = new Object() {};
    private static final Object LOCK_2 = new Object() {};
    static void foo() {
        synchronized(LOCK_1) {...}
    }
    static void fee() {
        synchronized(LOCK_1) {...}
    }
    static void fie() {
        synchronized(LOCK_2) {...}
    }
    static void fo() {
        synchronized(LOCK_2) {...}
    }
}

(for non-static methods, you would want to make the locks be non-static fields)

Scott Stanchfield