Announcement Announcement Module
Collapse
No announcement yet.
Synchronized keyword is not respected by annotated transaction handling. Page Title Module
Move Remove Collapse
X
Conversation Detail Module
Collapse
  • Filter
  • Time
  • Show
Clear All
new posts

  • Synchronized keyword is not respected by annotated transaction handling.

    Hi,

    I have the following method:

    Code:
        @Transactional(propagation = Propagation.REQUIRES_NEW)
        public synchronized Path2Uuid findOrCreate(String path) {
            Path2Uuid path2Uuid = _path2UuidDao.findByPath(path);
            if (path2Uuid != null) {
                return path2Uuid;
            }
            path2Uuid = new Path2Uuid(path, _uuidGenerator.getUuid(path));
            _path2UuidDao.makePersistent(path2Uuid);
            return path2Uuid;
        }
    If there is concurrent access to this method I get a race condition due to the fact, that the commit of the transaction is not part of the synchronized block. In the case above, two attempts are made to create the same path. The second thread entering the method don't find the path, due to the fact that the commit of the first thread has not been executed yet (as it is not synchronized). This leads to an ConstraintViolationException.

    I use a simple but ugly solution to this problem.

    Code:
    public synchronized Path2Uuid findOrCreate(String path) {
      findOrCreateForSynchronizedCommit(path);
    }
    @Transactional(propagation = Propagation.REQUIRES_NEW)
    public synchronized Path2Uuid findOrCreateForSynchronizedCommit(String path) {
            Path2Uuid path2Uuid = _path2UuidDao.findByPath(path);
            if (path2Uuid != null) {
                return path2Uuid;
            }
            path2Uuid = new Path2Uuid(path, _uuidGenerator.getUuid(path));
            _path2UuidDao.makePersistent(path2Uuid);
            return path2Uuid;
        }
    This way the transaction commit is synchronized for the caller of findOrCreate. It is ugly for two reasons. Annotated transaction handling force me to have two methods. The second method even has to be public as otherwise the transaction annotations are not considered.

    Maybe I don't understand something not properly. Please help me out. Otherwise I gonna file a Jira issue.

  • #2
    I think it isn't the way to handle synchronized access to the database, specify the correct serialization level and let the database handle it instead of (re)inventing your own.

    However I think your problem lies with the way proxies are being created, I think CGLIB of JdkDynamic proxies don't include the synchronized keyword.

    Comment


    • #3
      Originally posted by mdeinum View Post
      I think it isn't the way to handle synchronized access to the database, specify the correct serialization level and let the database handle it instead of (re)inventing your own.
      I agree.

      And combining both synchronization techniques is asking for a lot of trouble.

      If you want mutual exclusion for the transactions, you should use the serialized isolation level (like Marten suggested)

      What kind of database are you using?

      Comment


      • #4
        Thanks for your responses.

        Your points are valid. Using serializable would be theoretically the better option. But using:

        Code:
          @Transactional(propagation = Propagation.REQUIRES_NEW, isolation = Isolation.SERIALIZABLE)
           // Not using synchronized just enhances the number of raceconditions, even with isolation = serializable
          public synchronized Path2Uuid findOrCreate(String path) {
                Path2Uuid path2Uuid = _path2UuidDao.findByPath(path);
                if (path2Uuid != null) {
                    return path2Uuid;
                }
                path2Uuid = new Path2Uuid(path, _uuidGenerator.getUuid(path));
                _path2UuidDao.makePersistent(path2Uuid);
                return path2Uuid;
            }
        for the configuration:

        Code:
        <bean id="dataSource" class="org.logicalcobwebs.proxool.ProxoolDataSource">
            <property name="alias" value="pool1"/>
            <property name="driver" value="com.mysql.jdbc.Driver"/>
            <property name="driverUrl" value="jdbc:mysql://localhost/test"/>
            <property name="user" value="root"/>
            <property name="password" value=""/>
            <property name="delegateProperties" value="user=root"/>
            <property name="maximumConnectionCount" value="100"/>
            <property name="minimumConnectionCount" value="10"/>
            <property name="simultaneousBuildThrottle" value="100"/>
            <property name="houseKeepingTestSql" value="select CURRENT_DATE"/>
          </bean>
        	
            <bean id="hibernateProperties" 
              class="org.springframework.beans.factory.config.PropertiesFactoryBean">
              <property name="properties">
                 <props>     
                    <prop key="hibernate.cache.provider_class">org.hibernate.cache.NoCacheProvider</prop>    
                    <prop key="hibernate.dialect">org.hibernate.dialect.MySQLInnoDBDialect</prop>
                    <prop key="hibernate.transaction.factory_class">org.hibernate.transaction.JDBCTransactionFactory</prop>
                    <prop key="hibernate.cache.use_query_cache">false</prop>
                    <prop key="hibernate.connection.useUnicode">true</prop>
                    <prop key="hibernate.connection.characterEncoding">UTF8</prop>
                 </props>
              </property>
           </bean>
        did not solve the problem (using mysql db). I still get race conditions, that 2 threads want to create the path. I'm not keen on diving possibly for days into database/pool/spring/hibernate configuration issues.

        What dangers do you think lurk around when using the fix with the wrapped methods I'm using now (see my first posting)?

        However I think your problem lies with the way proxies are being created, I think CGLIB of JdkDynamic proxies don't include the synchronized keyword.
        Sure, it is an proxy issue. I found one reply by Rod Johnson, where he recommended using pool, if one has exactly such a problem. This doesn't appeal to me.

        Thanks

        - Hans

        Comment


        • #5
          The SERIALIZATION not working can be due to 2 things I guess. Either misconfiguration or MySQL doesn't support it.

          Chapter 10 of the MySQL guide covers setting the transaction level for MySQL.

          On the other hand I would like to see your complete transaction configuration? It appears as if you are relying on the Spring transaction manager but you have misconfigured it, judging by your hibernate properties.

          Comment


          • #6
            MySql supports serializable (that's what they say in the docu) via there InnoDB engine (which is used by default). Hsql for instance does not.

            Here is the rest of my spring config.

            Code:
            <bean id="sessionFactory"
                  class="org.springframework.orm.hibernate3.annotation.AnnotationSessionFactoryBean">
                  <property name="dataSource">
                     <ref bean="dataSource" />
                  </property>
                  <property name="hibernateProperties">
                     <ref bean="hibernateProperties" />
                  </property>
                  <!-- <property name="exposeTransactionAwareSessionFactory"
                     value="false">
                  </property>  -->
                  <property name="annotatedClasses">
                     <list>
                        <value>...</value>
                     </list>
                  </property>
               </bean>
               
               <bean id="transactionManager"
                  class="org.springframework.orm.hibernate3.HibernateTransactionManager">
                  <property name="sessionFactory" ref="sessionFactory" />
               </bean>
            
               <!-- enable the configuration of transactional behavior based on annotations -->
               <tx:annotation-driven proxy-target-class="true"/>
            Thanks a lot for your help.

            - Hans

            Comment


            • #7
              What kind of race condition do you get? Do you get an some sort of 'can't serialize access to' or 'a different transaction has modified the data, so I can't commit' ?

              This can be explained by the concurrency mechanism InnoDB uses (MVCC, could be seen as an optimistic locking mechanism).

              Comment


              • #8
                The findOrCreate(path) method works in a way, that the first thread calling it for a particular path, triggers a create. The other, later threads are supposed to find the row in the db, and return this row. If a second thread enters the findOrCreate method before the first thread did a commit, the second thread also tries to create the row. This leads to the following error (with isolation=serializable):

                Code:
                2007-03-20 16:48:28,297 [Thread-986] DEBUG ### general [krugle.rest.uuid.Path2UuidService] findOrCreateByPath Found Path2Uuid in db for path=path97 and uuid=lH1K5IwqUMtkdwzF
                2007-03-20 16:48:28,300 [Thread-987] DEBUG ### general [krugle.rest.uuid.Path2UuidService] findOrCreateByPath Entering findOrCreateByPath for path: path97
                Exception in thread "Thread-993" org.springframework.dao.DataIntegrityViolationException: Could not execute JDBC batch update; nested exception is org.hibernate.exception.ConstraintViolationException: Could not execute JDBC batch update
                Caused by: org.hibernate.exception.ConstraintViolationException: Could not execute JDBC batch update
                	at org.hibernate.exception.SQLStateConverter.convert(SQLStateConverter.java:71)
                	at org.hibernate.exception.JDBCExceptionHelper.convert(JDBCExceptionHelper.java:43)
                	at org.hibernate.jdbc.AbstractBatcher.executeBatch(AbstractBatcher.java:249)
                	at org.hibernate.engine.ActionQueue.executeActions(ActionQueue.java:235)
                	at org.hibernate.engine.ActionQueue.executeActions(ActionQueue.java:139)
                	at org.hibernate.event.def.AbstractFlushingEventListener.performExecutions(AbstractFlushingEventListener.java:298)
                	at org.hibernate.event.def.DefaultFlushEventListener.onFlush(DefaultFlushEventListener.java:27)
                	at org.hibernate.impl.SessionImpl.flush(SessionImpl.java:1000)
                	at org.hibernate.impl.SessionImpl.managedFlush(SessionImpl.java:338)
                	at org.hibernate.transaction.JDBCTransaction.commit(JDBCTransaction.java:106)
                	at org.springframework.orm.hibernate3.HibernateTransactionManager.doCommit(HibernateTransactionManager.java:562)
                	at org.springframework.transaction.support.AbstractPlatformTransactionManager.processCommit(AbstractPlatformTransactionManager.java:654)
                	at org.springframework.transaction.support.AbstractPlatformTransactionManager.commit(AbstractPlatformTransactionManager.java:624)
                	at org.springframework.transaction.interceptor.TransactionAspectSupport.commitTransactionAfterReturning(TransactionAspectSupport.java:307)
                	at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:117)
                	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185)
                	at org.springframework.aop.framework.Cglib2AopProxy$DynamicAdvisedInterceptor.intercept(Cglib2AopProxy.java:652)
                	at net.krugle.rest.uuid.Path2UuidService$$EnhancerByCGLIB$$6a5ec5c5.findOrCreateByPath(<generated>)
                	at net.krugle.rest.uuid.ConcTest$1.run(ConcTest.java:28)
                	at java.lang.Thread.run(Thread.java:613)
                Caused by: java.sql.BatchUpdateException: Duplicate entry 'path97' for key 2
                	at com.mysql.jdbc.ServerPreparedStatement.executeBatch(ServerPreparedStatement.java:665)
                	at sun.reflect.GeneratedMethodAccessor45.invoke(Unknown Source)
                	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
                	at java.lang.reflect.Method.invoke(Method.java:585)
                	at org.logicalcobwebs.proxool.ProxyStatement.invoke(ProxyStatement.java:100)
                	at org.logicalcobwebs.proxool.ProxyStatement.intercept(ProxyStatement.java:57)
                	at $java.sql.Statement$$EnhancerByProxool$$4c4c78b3.executeBatch(<generated>)
                	at org.hibernate.jdbc.BatchingBatcher.doExecuteBatch(BatchingBatcher.java:48)
                	at org.hibernate.jdbc.AbstractBatcher.executeBatch(AbstractBatcher.java:242)
                	... 17 more
                Thanks

                - Hans

                Comment


                • #9
                  I'm having the same issue and posted http://stackoverflow.com/questions/15158333/are-spring-proxy-calls-also-synchronized-if-the-target-class-methods-are-synchro . thought of going with the same solution you've used .. overlaying another method and making that method synchronized.. did you find any other solution ..?

                  Originally posted by hans_d View Post
                  Hi,

                  I have the following method:

                  Code:
                      @Transactional(propagation = Propagation.REQUIRES_NEW)
                      public synchronized Path2Uuid findOrCreate(String path) {
                          Path2Uuid path2Uuid = _path2UuidDao.findByPath(path);
                          if (path2Uuid != null) {
                              return path2Uuid;
                          }
                          path2Uuid = new Path2Uuid(path, _uuidGenerator.getUuid(path));
                          _path2UuidDao.makePersistent(path2Uuid);
                          return path2Uuid;
                      }
                  If there is concurrent access to this method I get a race condition due to the fact, that the commit of the transaction is not part of the synchronized block. In the case above, two attempts are made to create the same path. The second thread entering the method don't find the path, due to the fact that the commit of the first thread has not been executed yet (as it is not synchronized). This leads to an ConstraintViolationException.

                  I use a simple but ugly solution to this problem.

                  Code:
                  public synchronized Path2Uuid findOrCreate(String path) {
                    findOrCreateForSynchronizedCommit(path);
                  }
                  @Transactional(propagation = Propagation.REQUIRES_NEW)
                  public synchronized Path2Uuid findOrCreateForSynchronizedCommit(String path) {
                          Path2Uuid path2Uuid = _path2UuidDao.findByPath(path);
                          if (path2Uuid != null) {
                              return path2Uuid;
                          }
                          path2Uuid = new Path2Uuid(path, _uuidGenerator.getUuid(path));
                          _path2UuidDao.makePersistent(path2Uuid);
                          return path2Uuid;
                      }
                  This way the transaction commit is synchronized for the caller of findOrCreate. It is ugly for two reasons. Annotated transaction handling force me to have two methods. The second method even has to be public as otherwise the transaction annotations are not considered.

                  Maybe I don't understand something not properly. Please help me out. Otherwise I gonna file a Jira issue.

                  Comment


                  • #10
                    As mentioned in this thread don't use synchronized to limit access to a single thread, it is really a bad idea.

                    Also your second solution isn't going to work as the @Transactional is pretty much useless (unless you use a classbased proxy).

                    So use a correct isolation level (SERIALIZABLE) for your database access instead of a synchronized keyword.

                    Comment

                    Working...
                    X