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
}
This question is related to
java
multithreading
hibernate
concurrency
synchronization
If it is something to do with the data in your database, why not utilize database isolation locking to achieve?
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)
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
To answer your question, yes it does: your synchronized
method cannot be executed by more than one thread at a time.
static synchronized
means holding lock on the the class's Class
object
where as
synchronized
means holding lock on that class's object itself. That means, if you are accessing a non-static synchronized method in a thread (of execution) you still can access a static synchronized method using another thread.
So, accessing two same kind of methods(either two static or two non-static methods) at any point of time by more than a thread is not possible.
synchronized
Java keyword worksWhen you add the synchronized
keyword to a static method, the method can only be called by a single thread at a time.
In your case, every method call will:
SessionFactory
Session
However, these were your requirements:
getObjectById
being called for all classes when it is called by a particular classSo, even if the getObjectById
method is thread-safe, the implementation is wrong.
SessionFactory
best practicesThe SessionFactory
is thread-safe, and it's a very expensive object to create as it needs to parse the entity classes and build the internal entity metamodel representation.
So, you shouldn't create the SessionFactory
on every getObjectById
method call.
Instead, you should create a singleton instance for it.
private static final SessionFactory sessionFactory = new Configuration()
.configure()
.buildSessionFactory();
Session
should always be closedYou didn't close the Session
in a finally
block, and this can leak database resources if an exception is thrown when loading the entity.
According to the Session.load
method JavaDoc might throw a HibernateException
if the entity cannot be found in the database.
You should not use this method to determine if an instance exists (use
get()
instead). Use this only to retrieve an instance that you assume exists, where non-existence would be an actual error.
That's why you need to use a finally
block to close the Session
, like this:
public static synchronized Object getObjectById (Class objclass, Long id) {
Session session = null;
try {
session = sessionFactory.openSession();
return session.load(objclass, id);
} finally {
if(session != null) {
session.close();
}
}
}
In your case, you wanted to make sure only one thread gets access to that particular entity.
But the synchronized
keyword only prevents two threads from calling the getObjectById
concurrently. If the two threads call this method one after the other, you will still have two threads using this entity.
So, if you want to lock a given database object so no other thread can modify it, then you need to use database locks.
The synchronized
keyword only works in a single JVM. If you have multiple web nodes, this will not prevent multi-thread access across multiple JVMs.
What you need to do is use LockModeType.PESSIMISTIC_READ
or LockModeType.PESSIMISTIC_WRITE
while applying the changes to the DB, like this:
Session session = null;
EntityTransaction tx = null;
try {
session = sessionFactory.openSession();
tx = session.getTransaction();
tx.begin();
Post post = session.find(
Post.class,
id,
LockModeType.LockModeType.PESSIMISTIC_READ
);
post.setTitle("High-Performance Java Perisstence");
tx.commit();
} catch(Exception e) {
LOGGER.error("Post entity could not be changed", e);
if(tx != null) {
tx.rollback();
}
} finally {
if(session != null) {
session.close();
}
}
So, this is what I did:
EntityTransaction
and started a new database transactionPost
entity while holding a lock on the associated database recordPost
entity and committed the transactionException
being thrown, I rolled back the transactionStatic methods use the class as the object for locking, which is Utils.class for your example. So yes, it is OK.
Source: Stackoverflow.com