Let's say I have a java class with a method like this (just an example)
@Transactional
public synchronized void onRequest(Request request) {
if (request.shouldAddBook()) {
if (database.getByName(request.getBook().getName()) == null) {
database.add(request.getBook());
} else {
throw new Exception("Cannot add book - book already exist");
}
} else if (request.shouldRemoveBook()) {
if (database.getByName(request.getBook().getName()) != null) {
removeBook();
} else {
throw new Exception("Cannot remove book - book doesn't exist");
}
}
}
Say this book gets removed and then re-added with a new author or other minor change, so this method might get called twice very fast from another system, first to remove the Book, then to add the same Book back (with some new details).
To handle this we might try (like I did) to add the above @Transactional code, and then also 'synchronized' when the @Transactional doesn't work. But strangely it fails on the second call with
"Cannot add book - book already exist".
I spent a lot of time trying to figure this out, so thought I'd share the answer.
When removing and immediately adding back a Book, if we have no "@Transactional" or "synchronized" we are starting with this thread execution:
T1: |-----remove book----->
T2: |-------add book------->
The synchronized keyword makes sure that the method can only be run by one thread at a time. This means execution becomes this:
T1: |-----remove book-----> T2: |--------add book------>
The @Transactional annotation is an Aspect, and what it does is that it creates a proxy java class around your class, adds some code (begin transaction) to it before the method call, calls the method and then calls some other code (commit transaction). So the first thread now looks like this:
T1: |--Spring begins transaction--|-----remove book----- |--Spring commits transaction --->
or shorter: T1: |-B-|-R-|-C-->
and the second thread like this:
T2: |--Spring begins transaction--|-------add book------- |--Spring commits transaction --->
T2: |-B-|-A-|-C-->
Notice that the @Transactional annotation only locks the same entity in a database from being modified concurrently. Since we are adding a different entity (but with the same book name), it doesn't do much good. But it still shouldn't HURT right?
WELL HERE IS THE FUN PART:
The transaction code that Spring adds is not part of the synchronized method, so the T2 thread can actually start its method before the "commit" code is finished running, right after the first method call is done. Like this:
T1: |-B-|-R-|-C--|-->
T2: |-B------|-A-|-C-->
So. when the "add" method reads the database, the remove code has been run, but NOT the commit code, so it still finds the object in the database and throws the error. milliseconds later it will be gone from the database though.
Removing the @Transactional annotation would make the synchronized keyword work as expected, although this is not a good solution as others have mentioned. Removing synchronized and fixing the @Transactional annotation is a better solution.