Be careful with aspects (AOP) and synchronization

The problem
Recently I encountered a problem while working with Spring Aspects and a synchronized block of code. After some digging I came to a conclusion: Don't forget to consider the aspects that are applied to a method while working in a multi-threaded environment. Let me explain.

The project I'm working on uses transactional behavior provided by the spring framework. This behavior is applied by AspectJ using the @Transactional annotation at compile time. I have a method that needs to be synchronized, the problem is that there are two beans that inherits from a common base class that implements the synchronized method, so there are two instances that need to be synchronized. The way we solved the problem is by using a synchronized block with an shared lock object. The class structure is show here.

After some time I found that there were some inconsistent data. There problem was that for some reason the code inside the synchronized block appeared to be executed by two different threads at the same time (no way!!!). But as I supposed in the first place it wasn't the case. The real problem was that the method was not "fully" synchronized, I mean, not all the statements in the method were inside the synchronized block. The statements applied by the @Transactional aspect were out of the block, meaning that while one thread could be after the block (i.e. committing the transaction), another thread might already be inside the synchronized block.

Let me build and example (not exactly the same but it shows the problem). Suppose the class, the aspect and the main program.

We may expect the output to be like this (one invocation after the other)

Thread-0 Before perform
Thread-0 Begin method
Thread-0 End method
Thread-0 After perform
Thread-1 Before perform
Thread-1 Begin method
Thread-1 End method
Thread-1 After perform
...

But what really happens is something like this

Thread-0 Before perform
Thread-1 Before perform
Thread-0 Begin method
Thread-0 End method
Thread-1 Begin method
Thread-0 After perform
Thread-0 Before perform
Thread-1 End method
Thread-0 Begin method
Thread-0 End method
Thread-0 After perform

as you can see the execution of the method is synchronized (i.e. only one thread can be executing the block) while the statements in the aspect can be executed by multiple threads at the same time.

The solution

To solve this problem there are two options:

First, synchronize the code which invokes the perform method, as shown here. The problem with this approach is that you need to change every invocation of the method in the client code to include the synchronization block. If you forget to do this you will be in trouble.

Second, create a private method and decorate that one (with the aspect) instead of the public method as show here. As you can see this method allows to solve the problem without changing anything in the client.

A third option can be to refactor the code in a way the synchronization is not necessary, but for now let's keep it this way.