CODEWITHSUNDEEP
javamultithreadingservletsconcurrency
Sepfins
Sepfins

Reputation: 312

Preventing concurrent access to a method in servlet

I have a method in servlet that inserts tutoring bookings in database. This method has a business rule that checks if the tutor of this session is already busy in that date and hour. The code looks something like this :

class BookingService {
    public void insert(Booking t) {
        if(available(t.getTutor(), t.getDate(), t.getTime())) {
            bookingDao.insert(t);
        } else {
            // reject
        }
    }
}

The problem is that multiple users may simultaneously try to book the same tutor on the same date and time, and there is nothing that prevents them both to pass the test and insert their bookings. I've tried making insert() synchronized and using locks, but it doesn't work. How can I prevent concurrent access to this method?

Upvotes: 1

Views: 297

Answers (2)

Nathan Hughes
Nathan Hughes

Reputation: 96424

Using synchronized is an inadequate way to try to solve this problem:

First, you will have coded your application so that only one instance can be deployed at a time. This isn’t just about scaling in the cloud. It is normal for an IT department to want to stand up more than one instance of an application so that it is not a single point of failure (so that in case the box hosting one instance goes down the application is still available). Using static synchronized means that the lock doesn’t extend beyond one application classloader so multiple instances can still interleave their work in an error prone way.

If you should leave the project at some point, later maintainers may not be aware of this issue and may try to deploy the application in a way you did not intend. Using synchronized means you will have left a land mine for them to stumble into.

Second, using the synchronized block is impeding the concurrency of your application since only one thread can progress at a time.

So you have introduced a bottleneck, and at the same time cut off operations’ ability to work around the bottleneck by deploying a second instance. Not a good solution.

Since the posted code shows no signs of where transactions are, I’m guessing either each DAO creates its own transaction, or you’re connecting in autocommit mode. Databases provide transactions to help with this problem, and since the functionality is implemented in the database, it will work regardless of how many application instances are running.

An easy way to fix the problem which would avoid the above drawbacks would be to put the transaction at the service layer so that all the DAO calls would execute within the same transaction. You could have the service layer retrieve the database connection from a pool, start the transaction, pass the connection to each DAO method call, commit the transaction, then return the connection to the pool.

Upvotes: 1

Jason
Jason

Reputation: 11822

One way you could solve the problem is by using a synchronized block. There are many things you could choose as your locking object - for the moment this should be fine:

class BookingService {
    public void insert(Booking t) {
        synchronized(this) {
            if(available(t.getTutor(), t.getDate(), t.getTime())) {
                bookingDao.insert(t);
            } else {
                // reject
            }
        }
    }
}

If you have more than one instance of the servlet, then you should use a static object as a lock.

Upvotes: 0

Related Questions