Wait for Quartz jobs to finish before continuing shutdown of singleton beans [SPR-14806]

[spring-projects-issues]

Nicolas Labrot opened SPR-14806 and commented

This one originated as ticket in Spring Data but the essence seems to be this:

The SchedulerFactoryBean.stop() method calls the Quartz standby() method which does not wait for job completion. Thus, Spring continues its shutdown process and it starts to destroy singletons and especially the auditingHandler bean before the schedulerFactoryBean bean.

Original description

Hello,

When my application shutdown I got this exception:

org.springframework.transaction.TransactionSystemException: Could not commit JPA transaction; nested exception is javax.persistence.RollbackException: Error while committing the transaction
    at org.springframework.orm.jpa.JpaTransactionManager.doCommit(JpaTransactionManager.java:526)
    at org.springframework.transaction.support.AbstractPlatformTransactionManager.processCommit(AbstractPlatformTransactionManager.java:761)
    at org.springframework.transaction.support.AbstractPlatformTransactionManager.commit(AbstractPlatformTransactionManager.java:730)
    at org.springframework.transaction.interceptor.TransactionAspectSupport.commitTransactionAfterReturning(TransactionAspectSupport.java:485)
    at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:291)
    at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:96)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
    at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:655)
    at com.victorbuckservices.vbackbone.common.transaction.DoInTransactionService$$EnhancerBySpringCGLIB$$4c864b6e.execute(<generated>)
    at com.victorbuckservices.vbackbone.service.distribution.fax.service.DistributionFaxService.particularRslFolderPolling(DistributionFaxService.java:757)
    at com.victorbuckservices.vbackbone.service.distribution.fax.service.DistributionFaxService.rslPolling(DistributionFaxService.java:723)
    at com.victorbuckservices.vbackbone.service.distribution.fax.service.DistributionFaxService$$FastClassBySpringCGLIB$$7c544f09.invoke(<generated>)
    at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:204)
    at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:651)
    at com.victorbuckservices.vbackbone.service.distribution.fax.service.DistributionFaxService$$EnhancerBySpringCGLIB$$75a0516b.rslPolling(<generated>)
    at com.victorbuckservices.vbackbone.service.distribution.fax.quartz.PollingJob.execute(PollingJob.java:28)
    at org.quartz.core.JobRunShell.run(JobRunShell.java:202)
    at org.quartz.simpl.SimpleThreadPool$WorkerThread.run(SimpleThreadPool.java:573)
Caused by: javax.persistence.RollbackException: Error while committing the transaction
    at org.hibernate.jpa.internal.TransactionImpl.commit(TransactionImpl.java:94)
    at org.springframework.orm.jpa.JpaTransactionManager.doCommit(JpaTransactionManager.java:517)
    ... 17 common frames omitted
Caused by: org.springframework.beans.factory.BeanCreationNotAllowedException: Error creating bean with name 'jpaAuditingHandler': Singleton bean creation not allowed while the singletons of this factory are in destruction (Do not request a bean from a BeanFactory in a destroy method implementation!)

Some tasks must be finished prior to the shutdown and because spring is shutting down, it forbids call to DefaultSingletonBeanRegistry#getSingleton and thus AuditingEntityListener#touchForUpdate failed because he used an ObjectFactory<AuditingHandler>:

@PreUpdate
public void touchForUpdate(Object target) {
    if (handler != null) {
        handler.getObject().markModified(target);
    }
}

---

Issue Links:

• #19311 SchedulerFactoryBean's setOverwriteExistingJobs does not reliably work in a cluster

[spring-projects-issues]

Nicolas Labrot commented

Hello,

Is there a status about this issue?

Thanks

[spring-projects-issues]

Nicolas Labrot commented

This issue could be closed: it is not a spring data one but a spring SchedulerFactoryBean one.

The SchedulerFactoryBean#stop method calls the quartz standby method which does not wait for job completion. Thus, Spring continues its shutdown process and it starts to destroy singletons and especially the auditingHandler bean before the schedulerFactoryBean bean.

Thus when a job completes and commits, auditingHandler has already been destroyed.

[spring-projects-issues]

Oliver Drotbohm commented

Thanks for the heads up, Nicolas. Do we have an upstream ticket for Spring Framework actually? Otherwise I could just turn this one into a Spring Framework one.

[spring-projects-issues]

Nicolas Labrot commented

Yes please, but as an improvment

[spring-projects-issues]

Oliver Drotbohm commented

Do you have ticket ID handy? I couldn't find anything on a quick search.

[spring-projects-issues]

Nicolas Labrot commented

Sorry I mean you could turn this ticket into a Spring Framework one.

[spring-projects-issues]

Oliver Drotbohm commented

Moved, updated the description and assigned to Jürgen for further triage.

[spring-projects-issues]

Juergen Hoeller commented

Our Lifecycle.stop() method generally does not imply that all asynchronous tasks have ended. We could trigger a special variant of stop on application context shutdown, waiting for running tasks to finish. However, that would block the entire shutdown thread, even if all most/all other beans are unrelated. We could make that dependent on SchedulerFactoryBean's existing waitForJobsToCompleteOnShutdown flag, enforcing it on an early shutdown-related stop call already instead of just at the very end of the shutdown phase.

Alternatively, SchedulerFactoryBean would have to be marked as dependent on all affected beans which get used by Quartz jobs, so the application context would only destroy those beans once the scheduler itself got destroyed. This can be manually enforced through depends-on declarations already but seems hard to do by default.

[spring-projects-issues]

Nicolas Labrot commented

The shutdown thread is already blocked when using waitForJobsToCompleteOnShutdown. I'm not sure it would make a difference if quartz is shutdown during stop phase of during the destroy phase. A shutdownSchedulerOnStop might be enough.

depends-on could be easily forgotten and it would require training and peer review. I'd rather reverse the logic with the SmartLifecycle and the phased shutdown.

These thoughs seems applicable to ScheduledTaskRegistrar too which implements destroy and not the Lifecycle.

[spring-projects-issues]

Juergen Hoeller commented

There is a conceptual problem here in that stop() implies the ability to re-start(), and that translates to the Quartz standby() method very well, whereas shutdown is a terminal operation in Quartz. So for an early Quartz shutdown call, we'd have to introduce some early shutdown callback at the time of the stop signal... This feels rather convoluted and in conflict with the general lifecycle design in Spring as well as in Quartz.

As for ScheduledTaskRegistrar, like the Quartz SchedulerFactoryBean, the container knows about the tasks specified there and considers them as dependencies, understanding their transitive dependencies too. It won't destroy any of those until the registrar/scheduler itself gets destroyed, which leads to the right shutdown order in a straightforward dependency setup.

So from that perspective, the root of the problem remains that some scheduled tasks depend on the availability of further beans that they are not directly referring to. Since declared/resolved dependencies are taken into account already, it's only really about implicitly used beans such as the audit listener in your transaction setup. The proper solution seems to be to explicitly declare the SchedulerFactoryBean as dependent on such beans, or possibly simply the transaction manager bean as dependent on the audit listener bean... assuming that the scheduler is directly on indirectly dependent on the transaction manager bean already.

[spring-projects-issues]

Nicolas Labrot commented

I agree mostly with your comment. My cons are:

• I'm not sure I will not meet this issue because of new implicit dependencies.
• I have async tasks fired by quartz, rabbitmq and tomcat. I tend to favor explicit ordered start/stop to keep under control the task ending. Lifecycle fits this need.

What do you think of a waitForJobsToCompleteOnStop ?

For example here is an adapted code snippet from one of my integration test:

scheduler.standby();
if (waitForJobsToCompleteOnStop) {
  Stopwatch stopwatch = Stopwatch.createStarted();
  while (scheduler.getCurrentlyExecutingJobs().size() != 0 && stopwatch.elapsed(TimeUnit.MILLISECONDS) < 5000) {
    Thread.sleep(100);
  }
}

Further investigation may be need in order to check that after the scheduler.standby() no trigger can be fired.

[spring-projects-issues]

Nicolas Labrot commented

After a scheduler.standby(), triggers can be fired when the QuartzSchedulerThread is processing the trigger list.

I'm not sure that explicit bean dependency will solve the issue because I'm using a AutowiringSpringBeanJobFactory. Race condition may occur between stop, the job and destroy.

I have created this quartz issue Add a method to return true when the QuartzSchedulerThread is actually paused and created a custom SchedulerFactoryBean.

Thanks

[spring-projects-issues]

Bulk closing outdated, unresolved issues. Please, reopen if still relevant.