CODEWITHSUNDEEP
javamultithreadingconcurrency
karoma
karoma

Reputation: 1558

Ensuring only one thread is in a class

I have a DAO class which has multiple methods which update DB tables. These methods are called in many different places throughout the codebase, in many different orders.

public class Dao {
  public synchronized updateA()
  public synchronized updateB()
  public synchronized updateC()
  public synchronized getA()
  public synchronized getB()
  public synchronized getC()
}

I'm having an issue where Class1 wants to call getA(), getB(), and getC() in sequence. The three tables A, B, and C are related, so I need to get their state synchronized at one point in time.

However, after Class1 calls getA() and before it calls getB(), Class2 on another thread jumps in and calls updateB(), which breaks things.

Is it possible to lock the entire DAO class while any thread is in there, and only unlock it when it's finished?

I've tried putting a static ReentrantLock in the DAO class and locking it from Class1, but I'm not sure how to go from there.

I've gotten as far as the following:

public class Class1 {
  public void check() {
    dao.daoLock.lock();
    dao.getA();
    dao.getB();
    dao.getC();
    dao.daoLock.unlock();
  }
}

public class Dao {
  public static final daoLock = new ReentrantLock();
  public synchronized updateA() {
    daoLock.lock();
    // Do stuff...
  }
  public synchronized updateB() {
    daoLock.lock();
    // Do stuff...
  }
  public synchronized updateC() {
    daoLock.lock();
    // Do stuff...
  }
  public synchronized getA() {
    daoLock.lock();
    // Do stuff...
  }
  public synchronized getB() {
    daoLock.lock();
    // Do stuff...
  }
  public synchronized getC() {
    daoLock.lock();
    // Do stuff...
  }
}

I'm unsure about the location of the unlock. If I put it in each the DAO class methods it's going to let other threads in isn't it?

Is there a better solution here?

Upvotes: 0

Views: 59

Answers (2)

GhostCat
GhostCat

Reputation: 140457

Besides the good answer from Thomas, there is another aspect here worth looking at: interfaces should be written in a way that makes it easy to do the right thing; and hard to do the wrong thing.

Meaning: if some operation requires you to call a(), b(), c() in sequence, and in a "overall locked manner", then: instead of having a, b, c on your interface, you provide exactly one method abc() ... that acquires the lock, calls a,b,c and releases the lock.

So, you should step back and have a look at the interface of your class from a SOLID perspective; or more specific - considering it from the "single responsibility principle" side of things.

In other words: having one class that provides a lot of completely "disjunct" methods; where some of them even need to be "used together, in a very specific manner" that is a clear design smell; so another indication that you better step back and review this design.

Upvotes: 1

Thomas
Thomas

Reputation: 181785

You're trying to synchronise access to the database on the client side, which is fundamentally the wrong place to do it. What if another client did a write in the meantime?

It's better to leave this kind of synchronisation to the database itself, using transactions (MySQL, PostgreSQL).

Even if you don't have multiple clients and never will, using transactions is a better solution. Otherwise, all your threads will be blocked on each other even if they are just doing reads, which in principle can happen concurrently just fine.

Upvotes: 3

Related Questions