views:

284

answers:

2

My colleague and I have a web application that uses Spring 3.0.0 and JPA (hibernate 3.5.0-Beta2) on Tomcat inside MyEclipse. One of the data structures is a tree. Just for fun, we tried stress-testing the "insert node" operation with JMeter, and found a concurrency problem. Hibernate reports finding two entities with the same private key, just after a warning like this:

 WARN  [org.hibernate.engine.loading.LoadContexts] fail-safe cleanup (collections) : ...

It is quite easy to see how such problems might occur if multiple threads call the insert() method concurrently.

My servlet A calls a service layer object B.execute(), which then calls a lower layer object C.insert(). (The real code is too big to post, so this is somewhat abridged.)

Servlet A:

  public void doPost(Request request, Response response) {
    ...
    b.execute(parameters);
    ...
  }

Service B:

  @Transactional //** Delete this line to fix the problem.
  public synchronized void execute(parameters) {
    log("b.execute() starting. This="+this);
    ...
    c.insert(params);
    ...
    log("b.execute() finishing. This="+this);
  }

Sub-service C:

  @Transactional
  public void insert(params) {
    ...
    // data structure manipulation operations that should not be 
    // simultaneous with any other manipulation operations called by B.
    ...
  }

All my state-change calls go through B, so I decided to make B.execute() synchronized. It was already @Transactional, but it's actually the business logic that needs to be synchronized, not just the persistence, so that seems reasonable.

My C.insert() method was also @Transactional. But since the default transaction propagation in Spring seems to be Required, I don't think there was any new transaction being created for C.insert().

All components A, B, and C are spring-beans, and hence singletons. If there really is only one B object, then I conclude that it shouldn't be possible for more than one threat to execute b.execute() at a time. When the load is light, only a single thread is used, and this is the case. But under load, additional threads get involved, and I see several threads print "starting" before the first one prints "finishing". This seems to be a violation of the synchronized nature of the method.

I decided to print the this in the log messages to confirm whether there was only a single B object. All the log messages do show the same object id.

After much frustrating investigation, I discovered that removing the @Transactional for B.execute() solves the problem. With that line gone, I can have lots of threads, but I always see a "starting" followed by a "finishing" before the next "starting" (and my data structures stay intact). Somehow, the synchronized only seems to work when the @Transactional is not present. But I don't understand why. Can anyone help? Any tips as to how to look into this further?

In stack traces, I can see that there's an aop/cglib proxy generated in between A.doPost() and B.execute() - and also between B.execute() and C.insert(). I wonder whether somehow the proxy's construction might ruin the synchronized behaviour.

+1  A: 

Synchronized keyword requires, as you stated, that the object involved is always the same. I haven't observed the above mentioned behaviour myself but your suspect might just be correct one.

Have you tried logging out b from doPost -method? If that is different every time, then there is some spring magic with AOP/cglib proxies going on.

Anyway, I wouldn't rely on the syncronized keyword but use something like ReentrantLock from java.util.concurrent.locks to ensure the synchronization behavior instead, as your b object is always same regardless of possible multiple cglib proxies.

plouh
Thanks Plouh. I wasn't aware of ReentrantLock - I'll take a look.
John
A: 

I had the same problem, ReentrantLock seemed to do the trick. Thanks John for the post and Plouh for the answer

tommy