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

Scheduled tasks aren't cancelled on ScheduledTaskRegistrar destruction anymore [SPR-14286] #18858

Closed
spring-projects-issues opened this issue May 18, 2016 · 4 comments
Assignees
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented May 18, 2016

Romain Moreau opened SPR-14286 and commented

With Spring 4.3.0.RC1, when ScheduledTaskRegistrar was destroyed, scheduled futures were cancelled correctly.

Now, with Spring 4.3.0.RC2, when ScheduledTaskRegistrar is destroyed, scheduled tasks aren't cancelled because the scheduledTasks collection used to iterate the tasks to cancel is empty.

Here's below the code for a quick and dirty reproduction case.
To run with Spring 4.3.0.RC1, replace the spring-boot-starter-parent version with 1.4.0.M2.
With Spring 4.3.0.RC1, you'll see in the log "1 scheduled found" as expected.
With Spring 4.3.0.RC2, you'll see in the log "0 scheduled found" (scheduledTasks set is empty).

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>
  <parent>
    <groupId>org.springframework.boot</groupId>
    <artifactId>spring-boot-starter-parent</artifactId>
    <version>1.4.0.M3</version>
  </parent>
  <artifactId>scheduled-repro</artifactId>
  <properties>
    <!-- Needed because of a Maven issue on my side, not affecting reproduction case -->
    <spring-integration.version>4.2.6.RELEASE</spring-integration.version>
  </properties>
  <dependencies>
    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter</artifactId>
    </dependency>
  </dependencies>
  <build>
    <plugins>
      <plugin>
        <groupId>org.springframework.boot</groupId>
        <artifactId>spring-boot-maven-plugin</artifactId>
      </plugin>
    </plugins>
  </build>
  <repositories>
    <repository>
      <id>spring-milestones</id>
      <url>http://repo.spring.io/milestone</url>
      <releases>
        <enabled>true</enabled>
      </releases>
    </repository>
  </repositories>
  <pluginRepositories>
    <pluginRepository>
      <id>spring-milestones</id>
      <url>http://repo.spring.io/milestone</url>
      <releases>
        <enabled>true</enabled>
      </releases>
    </pluginRepository>
  </pluginRepositories>
</project>
package scheduledrepro;

import org.springframework.beans.factory.annotation.*;
import org.springframework.boot.*;
import org.springframework.boot.autoconfigure.*;
import org.springframework.scheduling.annotation.*;
import org.springframework.scheduling.config.*;
import org.springframework.scheduling.concurrent.*;
import org.springframework.context.annotation.*;
import org.springframework.boot.context.event.*;
import org.springframework.context.event.*;
import org.springframework.util.*;
import java.lang.*;
import java.lang.reflect.*;
import java.util.*;
import org.slf4j.*;

@EnableScheduling
@SpringBootApplication
public class ScheduledReproApplication implements SchedulingConfigurer {
 private static final Logger LOGGER = LoggerFactory.getLogger(ScheduledReproApplication.class);

 private ScheduledTaskRegistrar taskRegistrar;

 @Override
 public void configureTasks(ScheduledTaskRegistrar taskRegistrar) {
  this.taskRegistrar = taskRegistrar;
  taskRegistrar.setScheduler(threadPoolTaskScheduler());
 }

 @Bean
 public ThreadPoolTaskScheduler threadPoolTaskScheduler() {
  return new ThreadPoolTaskScheduler();
 }

 @org.springframework.context.event.EventListener
 public void applicationReadyEvent(ApplicationReadyEvent applicationReadyEvent) throws Exception {
  Field field = ReflectionUtils.findField(ScheduledTaskRegistrar.class, "scheduledTasks"); // Spring 4.3.0.RC2
  if (field == null) {
   field = ReflectionUtils.findField(ScheduledTaskRegistrar.class, "scheduledFutures"); // Spring 4.3.0.RC1
  }
  field.setAccessible(true);
  Set < ? > scheduledTasks = (Set < ? > ) field.get(taskRegistrar);
  LOGGER.info("{} scheduled found", scheduledTasks.size());
 }

 @Scheduled(cron = "* * * * * *")
 public void scheduled() {
  LOGGER.info("Scheduled");
 }

 public static void main(String[] args) throws Exception {
  SpringApplication.run(ScheduledReproApplication.class, args);
 }
}

Affects: 4.3 RC2

Issue Links:

  • #16830 ScheduledAnnotationBeanPostProcessor should unregister tasks on destruction of individual beans

Referenced from: commits 3576ff0

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 29, 2016

Juergen Hoeller commented

Indeed, as of 4.3 RC2, ScheduledAnnotationBeanPostProcessor tracks its scheduled tasks internally, with ScheduledTaskRegistrar.destroy() not affecting those anymore. However, tasks should still get destroyed on shutdown through ScheduledAnnotationBeanPostProcessor's destruction callbacks. So are your beans not getting destroyed at all, or are you simply not seeing the expected effect on ScheduledTaskRegistrar.destroy() now (which is by design)?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 29, 2016

Romain Moreau commented

I looked at ScheduledAnnotationBeanPostProcessor and any Set that is a value of the scheduledTasks map will only contain one task associated to the last @Scheduled annotated method of a bean because when there is a previous Set already created, it is always overwritten on line 400.

For instance, given the following class, Spring 4.3.RC2 will only cancel the task associated to the scheduled3 method, the tasks associated to the scheduled1 and the scheduled2 methods won't be cancelled.

@Component
public class Scheduler {
	private static final Logger LOGGER = LoggerFactory.getLogger(Scheduler.class);
	
	@Scheduled(cron = "*/30 * * * * *")
	public void scheduled1() {
		LOGGER.info("1");
	}

	@Scheduled(cron = "0 0 0 * * *")
	public void scheduled2() {
		LOGGER.info("2");
	}

	@Scheduled(cron = "15 * * * * *")
	public void scheduled3() {
		LOGGER.info("3");
	}
}
@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 30, 2016

Juergen Hoeller commented

Good catch! Addressed in master now; please give the upcoming 4.3.0.BUILD-SNAPSHOT a try if you have the chance...

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented May 31, 2016

Romain Moreau commented

I tried the latest SNAPSHOT build with your changes and it works for me, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants