Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression in transaction rollback handling [SPR-11406] #16033

Closed
spring-projects-issues opened this issue Feb 9, 2014 · 14 comments
Closed

Regression in transaction rollback handling [SPR-11406] #16033

spring-projects-issues opened this issue Feb 9, 2014 · 14 comments
Assignees
Labels
in: data status: declined

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Feb 9, 2014

Alex opened SPR-11406 and commented

In spring 4, the handling of transaction boundaries between function call has changed considerably and code that worked fine in 3.x now stopped working.

Here is an example where a user function in a service throws an exception when a certain condition is met (in this case when the object is not found in the database).

The caller catches the exception and goes on but the transaction has already been rollback and the subsequent write on the database fails.

A proof of concept follows, where a rest service is called and this method use a service that raises an exception when the requested object is not found.

The repository is just a plain default spring-jpa repository.

@RestController
@RequestMapping("/test")
@Transactional
public class TestController {
  @Autowired
  private userRepository userRepository;
  @Autowired
  private TestService testService;

  @RequestMapping("/newUser/{name}")
  public User testNewUser(@PathVariable String name) {
    try {
      testService.findUser(name);
      logger.debug("user already exists: " + name);
      return null;
    } catch (UserNotFoundException ex) {
    }
    User user = new User();
    user.setName(name);
    userRepository.save(user);
    return user;
  }
}
@Service
@Transactional
public class TestService {

  @Autowired
  private userRepository userRepository;

  public User findUser(String name) {
    User user = userRepository.findByName(name);
    if (user == null)
      throw new UserNotFoundException(name);
    return user;
  }

}

Affects: 4.0 GA, 4.0.1

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 9, 2014

Juergen Hoeller commented

I'm not aware of any changes on our side that would cause such a change of behavior. It might be an accidental side effect on our side, of course, but there's also a chance that it is caused by some co-upgraded third-party library such as your persistence provider.

So please state the exact combination of project versions where it worked for you before, as well as your current combination of versions where it doesn't work anymore. We'll be researching this ASAP then.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 9, 2014

Alex commented

I'm unable to reproduce the problem with a project from scratch.

I'll try to slim down my app up to a point to either understand the problem or upload it here.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 9, 2014

Alex commented

Repository pull request: spring-attic/spring-framework-issues#65

I've reduced to absolute minimum the project and the problem still stands, I can't even tell the difference between this one and the one made from scratch from one of your templates, but the behaviour is different.

I think the only difference is the initialization code: I use a WebApplicationInitializer while you use the "old" web.xml syntax.

You can trigger the problem accessing an url like this:

http://localhost:8080/SPR-11406/newUser/alex

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 9, 2014

Sam Brannen commented

Hi Alex,

I just took a quick glance at the code you submitted in your pull request (admittedly without having run it), and it looks to me like you should just delete the @Transactional declaration on your controller. Your TestService is already annotated with @Transactional, so you're likely not going to want to have a transaction started by Spring when your controller handler method is invoked.

So try removing the @Transactional declaration from your controller and see if you then experience the desired behavior.

Regards,

Sam

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 9, 2014

Sam Brannen commented

Alex,

The project you have submitted unfortunately does not deploy.

It fails with the following stack trace.

SEVERE: StandardWrapper.Throwable
org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'entityManagerFactory' defined in ServletContext resource [/WEB-INF/applicationContext.xml]: Cannot resolve reference to bean 'dataSource' while setting bean property 'dataSource'; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'dataSource' defined in ServletContext resource [/WEB-INF/applicationContext.xml]: Error setting property values; nested exception is org.springframework.beans.PropertyBatchUpdateException; nested PropertyAccessExceptions (1) are:
PropertyAccessException 1: org.springframework.beans.MethodInvocationException: Property 'driverClassName' threw exception; nested exception is java.lang.IllegalStateException: Could not load JDBC driver class [com.mysql.jdbc.Driver]
	at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveReference(BeanDefinitionValueResolver.java:328)
	at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveValueIfNecessary(BeanDefinitionValueResolver.java:107)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyPropertyValues(AbstractAutowireCapableBeanFactory.java:1456)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.populateBean(AbstractAutowireCapableBeanFactory.java:1197)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:537)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:475)
	at org.springframework.beans.factory.support.AbstractBeanFactory$1.getObject(AbstractBeanFactory.java:304)
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:228)
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:300)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:195)
	at org.springframework.context.support.AbstractApplicationContext.getBean(AbstractApplicationContext.java:973)
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:750)
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:482)
	at org.springframework.web.servlet.FrameworkServlet.configureAndRefreshWebApplicationContext(FrameworkServlet.java:658)
	at org.springframework.web.servlet.FrameworkServlet.initWebApplicationContext(FrameworkServlet.java:530)
	at org.springframework.web.servlet.FrameworkServlet.initServletBean(FrameworkServlet.java:484)
	at org.springframework.web.servlet.HttpServletBean.init(HttpServletBean.java:136)
	at javax.servlet.GenericServlet.init(GenericServlet.java:160)
	at org.apache.catalina.core.StandardWrapper.initServlet(StandardWrapper.java:1280)
	at org.apache.catalina.core.StandardWrapper.load(StandardWrapper.java:1091)
	at org.apache.catalina.core.StandardContext.loadOnStartup(StandardContext.java:5198)
	at org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5481)
	at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:150)
	at org.apache.catalina.core.ContainerBase.addChildInternal(ContainerBase.java:901)
	at org.apache.catalina.core.ContainerBase.addChild(ContainerBase.java:877)
	at org.apache.catalina.core.StandardHost.addChild(StandardHost.java:634)
	at org.apache.catalina.startup.HostConfig.deployWAR(HostConfig.java:1074)
	at org.apache.catalina.startup.HostConfig$DeployWar.run(HostConfig.java:1858)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:744)
Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'dataSource' defined in ServletContext resource [/WEB-INF/applicationContext.xml]: Error setting property values; nested exception is org.springframework.beans.PropertyBatchUpdateException; nested PropertyAccessExceptions (1) are:
PropertyAccessException 1: org.springframework.beans.MethodInvocationException: Property 'driverClassName' threw exception; nested exception is java.lang.IllegalStateException: Could not load JDBC driver class [com.mysql.jdbc.Driver]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyPropertyValues(AbstractAutowireCapableBeanFactory.java:1493)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.populateBean(AbstractAutowireCapableBeanFactory.java:1197)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:537)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:475)
	at org.springframework.beans.factory.support.AbstractBeanFactory$1.getObject(AbstractBeanFactory.java:304)
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:228)
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:300)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:195)
	at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveReference(BeanDefinitionValueResolver.java:320)
	... 32 more
Caused by: org.springframework.beans.PropertyBatchUpdateException; nested PropertyAccessExceptions (1) are:
PropertyAccessException 1: org.springframework.beans.MethodInvocationException: Property 'driverClassName' threw exception; nested exception is java.lang.IllegalStateException: Could not load JDBC driver class [com.mysql.jdbc.Driver]
	at org.springframework.beans.AbstractPropertyAccessor.setPropertyValues(AbstractPropertyAccessor.java:108)
	at org.springframework.beans.AbstractPropertyAccessor.setPropertyValues(AbstractPropertyAccessor.java:62)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyPropertyValues(AbstractAutowireCapableBeanFactory.java:1489)
	... 40 more

The MySQL driver cannot be found, but even if it could, the application would still not deploy unless MySQL is installed and running on localhost and configured as on your machine. Thus, in order for us to help you better, please submit an update to your pull request that uses an embedded database instead of a local MySQL instance. For example, you might consider using Spring's <jdbc:embedded-database ... > support. ;)

Thanks,

Sam

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 9, 2014

Sam Brannen commented

Hi Alex,

I got your code working as I assume you expect it to.

Here's what you need to change...

1) Add the following to the <dependencies> section of your Maven pom.xml:

<dependency>
     <groupId>org.hsqldb</groupId>
     <artifactId>hsqldb</artifactId>
     <version>2.3.1</version>
</dependency>

2) Delete @Transactional from TestController.

3) Replace your existing applicationContext.xml with this:

<?xml version="1.0" encoding="UTF-8"?>
<beans
    xmlns="http://www.springframework.org/schema/beans"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns:context="http://www.springframework.org/schema/context"
    xmlns:tx="http://www.springframework.org/schema/tx"
    xmlns:jpa="http://www.springframework.org/schema/data/jpa"
    xmlns:jdbc="http://www.springframework.org/schema/jdbc"
    xsi:schemaLocation="
	http://www.springframework.org/schema/jdbc http://www.springframework.org/schema/jdbc/spring-jdbc.xsd
    http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd
    http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context.xsd
    http://www.springframework.org/schema/tx http://www.springframework.org/schema/tx/spring-tx.xsd
    http://www.springframework.org/schema/data/jpa http://www.springframework.org/schema/data/jpa/spring-jpa.xsd
">

    <context:annotation-config />

    <context:component-scan base-package="it.nibbles.test"/>

	<jdbc:embedded-database id="dataSource" />

	<bean id="jpaVendorAdapter" class="org.springframework.orm.jpa.vendor.HibernateJpaVendorAdapter">
		<property name="showSql" value="false" />
		<property name="generateDdl" value="true" />
		<property name="database" value="HSQL" />
	</bean>

	<bean id="entityManagerFactory" class="org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean">
		<property name="dataSource" ref="dataSource" />
		<property name="jpaVendorAdapter" ref="jpaVendorAdapter" />
		<property name="packagesToScan" value="it.nibbles.test.objs" />
	</bean>

	<bean id="transactionManager" class="org.springframework.orm.jpa.JpaTransactionManager">
		<property name="entityManagerFactory" ref="entityManagerFactory" />
		<property name="dataSource" ref="dataSource" />
	</bean>

    <tx:annotation-driven /> <!-- proxy-target-class="false" / -->

    <jpa:repositories base-package="it.nibbles.test.dao"
                    entity-manager-factory-ref="entityManagerFactory" transaction-manager-ref="transactionManager"/>

</beans>

4) Change the JPA mapping for User#id to @GeneratedValue(strategy = GenerationType.AUTO). (this may not actually be necessary, but it's a change I made along the way with the switch HSQL DB)

Once I made these changes and deployed the application to Tomcat 7.0.50, I see the following on the first request to http://localhost:8080/SPR-11406/newUser/alex:

New User id: 1

... and the following on all subsequent requests:

Existing User id: 1

This is what you wanted to see -- right?

Please let us know if this solves your problem!

Thanks,

Sam

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 9, 2014

Alex commented

Hello Sam,
using @Transactional on the controller is the whole point of this ticket, and of course it works if I make it not transactional.

Of course this example is as limited as possible to reproduce the issue: in the original one there is more logic on the controller method, so having it marked as transactional is imperative.

I've modified the pull request to use the embedded HSQL database, please tell me if you're able to reproduce the problem, thanks!

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 10, 2014

Sam Brannen commented

Alex, thanks for updating your pull request. I'll take another look and get back to you.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 10, 2014

Sam Brannen commented

Just to confirm...

When I deploy your updated example application running against Spring Framework 4.0.1.RELEASE, I get the following error on the initial request:

javax.persistence.RollbackException: Transaction marked as rollbackOnly
	org.hibernate.jpa.internal.TransactionImpl.commit(TransactionImpl.java:74)
	org.springframework.orm.jpa.JpaTransactionManager.doCommit(JpaTransactionManager.java:515)
	org.springframework.transaction.support.AbstractPlatformTransactionManager.processCommit(AbstractPlatformTransactionManager.java:757)
	org.springframework.transaction.support.AbstractPlatformTransactionManager.commit(AbstractPlatformTransactionManager.java:726)
	org.springframework.transaction.interceptor.TransactionAspectSupport.commitTransactionAfterReturning(TransactionAspectSupport.java:478)
	org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:272)
	org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:95)
	org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
	org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:643)
	it.nibbles.test.controllers.TestController$$EnhancerByCGLIB$$106b1f09.testNewUser(<generated>)
	sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	java.lang.reflect.Method.invoke(Method.java:483)
	org.springframework.web.bind.annotation.support.HandlerMethodInvoker.invokeHandlerMethod(HandlerMethodInvoker.java:175)
	org.springframework.web.servlet.mvc.annotation.AnnotationMethodHandlerAdapter.invokeHandlerMethod(AnnotationMethodHandlerAdapter.java:446)
	org.springframework.web.servlet.mvc.annotation.AnnotationMethodHandlerAdapter.handle(AnnotationMethodHandlerAdapter.java:434)
	org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:945)
	org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:876)
	org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:961)
	org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:852)
	javax.servlet.http.HttpServlet.service(HttpServlet.java:621)
	org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:837)
	javax.servlet.http.HttpServlet.service(HttpServlet.java:728)
	org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52)

However, this is to be expected, and I'll get to that in more detail later.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 10, 2014

Sam Brannen commented

To confirm further, I get the same exception when running the example application against Spring Framework 3.2.7.RELEASE.

So this is definitely not a regression from 3.2 to 4.0, and I assume that it is also not a regression against previous 3.1 or 3.0 releases.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 10, 2014

Sam Brannen commented

Hi Alex,

The unexpected behavior encountered in the example application (i.e., the aforementioned RollbackException) results from a misconfiguration of the application.

The message of the exception, "Transaction marked as rollbackOnly", spells it out rather explicitly.

TestService.findUser() is a transactional method (i.e., the class is annotated with @Transactional) that throws a UserNotFoundException if the user does not yet exist in the database (e.g., for the initial web request). Since UserNotFoundException is a subclass of RuntimeException, Spring's transaction management facilities automatically mark the current transaction for "rollback only".

This behavior is well documented in the Rolling back a declarative transaction section of the reference manual. Here's an excerpt:

The recommended way to indicate to the Spring Framework’s transaction infrastructure that a transaction’s work is to be rolled back is to throw an `Exception` from code that is currently executing in the context of a transaction. The Spring Framework’s transaction infrastructure code will catch any unhandled Exception as it bubbles up the call stack, and make a determination whether to mark the transaction for rollback.

In its default configuration, the Spring Framework’s transaction infrastructure code only marks a transaction for rollback in the case of runtime, unchecked exceptions; that is, when the thrown exception is an instance or subclass of `RuntimeException`. (Errors will also -- by default -- result in a rollback). Checked exceptions that are thrown from a transactional method do not result in rollback in the default configuration.

And here's another pertinent excerpt:

You can also specify no rollback rules, if you do not want a transaction rolled back when an exception is thrown.

So you have two options to address your issue.

Declare UserNotFoundException as a "Checked Exception"

One solution to your problem is to declare UserNotFoundException as a subclass of Exception instead of RuntimeException and then declare that TestService.findUser() throws UserNotFoundException. I have verified that this works against both Spring 3.2.7 and 4.0.1.

Configure Spring not to rollback for UserNotFoundException

An alternative solution is to ensure that the Spring-managed transaction will not be marked for rollback if a UserNotFoundException (as a subclass of RuntimeException) is thrown. You can achieve this with the following configuration for TestService.findUser().

@Transactional(noRollbackFor = UserNotFoundException.class)
public User findUser(String name)  {
    User user = userRepository.findByName(name);
    if (user == null) {
        throw new UserNotFoundException(name);
    }
    return user;
}

I have also verified this solution against both Spring 3.2.7 and 4.0.1.


Hopefully this clarifies what's going on behind the scenes!

Regards,

Sam

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 10, 2014

Sam Brannen commented

Resolving this issue as "Works as Designed".

Please see previous comments for details.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 10, 2014

Alex commented

Sam, thanks so much for the detailed information: since I made some other seemingly minor modifications to my code while upgrading to Spring 4 I've probably inputed to the upgrade the regression while probably the method passed from non-transactional to transactional.

I thought that unchecked exceptions would have popped out of the call stack and the transaction aborted (rolled back) only when the outer boundary would have been surpassed.

Thanks again and sorry for the noise.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 10, 2014

Sam Brannen commented

You're welcome, Alex.

We're just glad it was not an actual regression. ;-)

Cheers,

Sam

@spring-projects-issues spring-projects-issues added status: declined type: regression in: data labels Jan 11, 2019
@spring-projects-issues spring-projects-issues removed the type: regression label Jan 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data status: declined
Projects
None yet
Development

No branches or pull requests

2 participants