problem

In our project, there are two classes (schematic):

@Component
public class FatherScheduler {
    @Scheduled(cron = "0/10 * * * * ?")
    public void execute() {
        System.out.println(new Date() + " The classes that perform tasks are:" + this.getClass());
    }
}

@Component
public class SonSchedulerImpl extends FatherScheduler {
    @Override
    @Scheduled(cron = "1/10 * * * * ?")
    public void execute() {
        super.execute();
    }
}

This is a parent-child class.FatherScheduler defines a timed task and uses the Spring-Scheduler annotation to declare that it is scheduled to execute once every 10 seconds.SonScheduler Impl inherits FatherScheduler and overrides the annotations for the timed task.

In an online system, we already have a logically complete timer task parent class; the subclass only needs to modify an injection instance of the parent class and the cron expression.So there's a class structure like this.

We want the timed tasks defined by the parent class to start at 0/10/20/30...Seconds to start execution; Subclass timed tasks in 1/11/21/31...Seconds to start execution.There seems to be no problem with the code, but the actual result is as follows:

That is, parent timed tasks are actually scheduled to perform as we expect.But subclass timer tasks...Outside of our expectations, it makes one more schedule and has the same schedule as the parent class (red font part).Although we all do idempotent processing on timed tasks, even if we run a few more times, it is just a waste of point server performance, but the code should do and only do what we want to do, and should not do redundant things - otherwise, the "Skynet" system may be born someday.

Reason

From the code execution, it seems that Spring-Scheduler parsed the @Scheduled annotation on the subclass override method as well as the annotation on the parent method when registering a timed task for the subclass.But whether this is the case or not, you still need to find the code to register the timer task.

Find the @Scheduled annotation and see where it was handled in javadoc:

Processing of {@code @Scheduled} annotations is performed by registering a {@link ScheduledAnnotationBeanPostProcessor}. This can be done manually or, more conveniently, through the {@code <task:annotation-driven/>} element or @{@link EnableScheduling} annotation.

(This is also an inspiration: write Javadoc well.A good Javadoc is very convenient for people who use and maintain code.)

The definition and core processing code for Scheduled AnnotationBeanPostProcessor is as follows:

public class ScheduledAnnotationBeanPostProcessor implements BeanPostProcessor, Ordered,
      EmbeddedValueResolverAware, BeanFactoryAware, ApplicationContextAware,
      SmartInitializingSingleton, ApplicationListener<ContextRefreshedEvent>, DisposableBean {
    @Override
    public Object postProcessAfterInitialization(final Object bean, String beanName) {
       Class<?> targetClass = AopUtils.getTargetClass(bean);
       if (!this.nonAnnotatedClasses.contains(targetClass)) {
          final Set<Method> annotatedMethods = new LinkedHashSet<Method>(1);
          ReflectionUtils.doWithMethods(targetClass, new MethodCallback() {
             @Override
             public void doWith(Method method) throws IllegalArgumentException, IllegalAccessException {
                for (Scheduled scheduled :
                      AnnotationUtils.getRepeatableAnnotation(method, Schedules.class, Scheduled.class)) {
                   processScheduled(scheduled, method, bean);
                   annotatedMethods.add(method);
                }
             }
          });
          if (annotatedMethods.isEmpty()) {
             this.nonAnnotatedClasses.add(targetClass);
             if (logger.isDebugEnabled()) {
                logger.debug("No @Scheduled annotations found on bean class: " + bean.getClass());
             }
          }
          else {
             // Non-empty set of methods
             if (logger.isDebugEnabled()) {
                logger.debug(annotatedMethods.size() + " @Scheduled methods processed on bean '" + beanName +
                      "': " + annotatedMethods);
             }
          }
       }
       return bean;
    }               
}

In a nutshell, the core logic is in the ReflectionUtils.doWithMethods method.Inside this method is the following:

public static void doWithMethods(Class<?> clazz, ReflectionUtils.MethodCallback mc, ReflectionUtils.MethodFilter mf) {
    Method[] methods = getDeclaredMethods(clazz);
    Method[] var4 = methods;
    int var5 = methods.length;

    int var6;
    for(var6 = 0; var6 < var5; ++var6) {
        Method method = var4[var6];
        if (mf == null || mf.matches(method)) {
            try {
                mc.doWith(method);
            } catch (IllegalAccessException var9) {
                throw new IllegalStateException("Not allowed to access method '" + method.getName() + "': " + var9);
            }
        }
    }

    if (clazz.getSuperclass() != null) {
        doWithMethods(clazz.getSuperclass(), mc, mf);
    } else if (clazz.isInterface()) {
        Class[] var10 = clazz.getInterfaces();
        var5 = var10.length;

        for(var6 = 0; var6 < var5; ++var6) {
            Class<?> superIfc = var10[var6];
            doWithMethods(superIfc, mc, mf);
        }
    }

}

Alas~Otherwise, we find code within this class that recursively calls the parent class:

if (clazz.getSuperclass() != null) {
        doWithMethods(clazz.getSuperclass(), mc, mf);
    }

Combining the definition of the mc.doWith() method:

new MethodCallback() {
    @Override
    public void doWith(Method method) throws IllegalArgumentException, IllegalAccessException {
        for (Scheduled scheduled :
          AnnotationUtils.getRepeatableAnnotation(method, Schedules.class, Scheduled.class)) {
               processScheduled(scheduled, method, bean);
               annotatedMethods.add(method);
        }
    }
}

The cause of the problem is obvious:

When ScheduledAnnotationBeanPostProcessor processes a SonSchedulerImpl instance, it first finds the execute() method overridden by the subclass and the Scheduled annotation, and registers a timed task for it.Subsequently, following the same logic, the execute() method and the Scheduled annotation defined on its parent FatherScheduler are found, and a timed task is registered with the parent's configuration.Thus, two timer tasks are registered on the same bean instance, resulting in the same timer task being scheduled twice.

Solution

There are two solutions.

First of all...Do not comment @Scheduled on parent methods.In order to reuse the code as much as possible without annotating @Scheduled on the parent class, we finally changed the code to this:

public class FatherScheduler {
    
    public void execute() {
        System.out.println(new Date() + " The classes that perform tasks are:" + this.getClass());
    }
}

@Component
public class SonSchedulerImpl extends FatherScheduler {
    @Override
    @Scheduled(cron = "1/10 * * * * ?")
    public void execute() {
        super.execute();
    }
}

@Component
public class DaughterSchedulerImpl {
    @Scheduled(cron = "0/10 * * * * ?")
    public void execute() {
        super.execute();
    }
}

That is, the parent class only defines business logic and does not annotate @Scheduled.The two subclasses are annotated separately.

Another way to be more thorough is to upgrade the Spring version.This problem is currently known to occur in version 4.1.6.RELEASE; it has been fixed in the latest version of 5.1.8 RELEASE.When ReflectionUtils.doWithMethods is called in this version, a callback method like this is passed in:

ReflectionUtils.doWithMethods(currentHandlerType, method -> {
      Method specificMethod = ClassUtils.getMostSpecificMethod(method, targetClass);
      T result = metadataLookup.inspect(specificMethod);
      if (result != null) {
         Method bridgedMethod = BridgeMethodResolver.findBridgedMethod(specificMethod);
         if (bridgedMethod == specificMethod || metadataLookup.inspect(bridgedMethod) == null) {
            methodMap.put(specificMethod, result);
         }
      }
   }, ReflectionUtils.USER_DECLARED_METHODS);
}

Notice this line of code:

Method specificMethod = ClassUtils.getMostSpecificMethod(method, targetClass);

The purpose of this line of code is to find the method method it overrides from the targetClass.In the process where the problem occurs, the targetClass always points to the subclass SonSchedulerImpl; however, the method changes from the SonSchedulerImpl#execute() method to the FatherScheduler#execute() method with the recursive call to ReflectionUtils.doWithMethods.However, after processing with the ClassUtils.getMostSpecificMethod() method, the resulting specificMethod is still the SonSchedulerImpl#execute() method overridden by the subclass, not the native FatherScheduler#execute() method on the parent class.In this way, the subsequent processing will only register the timer tasks according to the @Scheduled annotation on the subclass method.

This is the second revelation: Framework tools should be upgraded in time to avoid stepping on holes already filled by others.