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 no longer works in case of multiple proxied target classes implementing the same interface [SPR-12709] #17306

Closed
spring-issuemaster opened this issue Feb 11, 2015 · 8 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Feb 11, 2015

Trent Summerfield opened SPR-12709 and commented

We had a number of @Scheduled tasks stop firing after upgrading spring 3.2.x. I have tracked down the problem to changes introduced by #16803

We have a point cut set as such

@Pointcut("execution(* com.example.Processor.process())")}}

where Processor is a simple interface

public interface Processor {
    void process();
}

The implementing class looks like

@Component
public class ProcessorImpl implements Processor {

    @Override
    @Scheduled(cron = "*/2 * * * * *")
    public void process() {
        System.out.println("It works!");
    }
}

This works as expected in 3.2.11, printing "It works!" every 2 seconds. However the scheduled task fails to fire since 3.2.12. I have tested this with aspectj versions 1.7.4 and 1.8.5 and both fail to work.

I have confirmed that reverting commit 37da70629f68b07d83d5c57abb74cecb7ecb358b, introduced by #16803, fixes this problem.


Affects: 3.2.12, 3.2.13

Issue Links:

  • #16803 Scheduled/JmsListenerAnnotationBeanPostProcessor needlessly scans every scoped instance

Referenced from: commits 1273c90, 3b8d878, f8a8ecd

Backported to: 3.2.14

1 votes, 7 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 12, 2015

Juergen Hoeller commented

This arguably wasn't ever meant to work: If an interface-based proxy hides underlying annotations, we're usually not discovering them for external invocation purposes... since it's not even guaranteed that the annotated method is visible through the interface. We have a few such inconsistencies in the framework, unfortunately, where invalid variants worked by accident.

As a general rule, you'll have to declare service-level annotations such that they are visible on the outer facade of a bean. So either declaring the annotation on the interface method or switching your proxy mode to target-class should do the job. Can you confirm that those options do address your problem? Just making sure that I'm not missing something...

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 12, 2015

Juergen Hoeller commented

Actually, on review, we do have specific support for @Scheduled methods hidden behind proxies, explicitly checking whether a corresponding method exists on the proxy and accepting such a scenario then. So even if I wouldn't recommend such an arrangement, it is a backwards compatibility issue since it has been explicitly supported before.

In fact, it still is explicitly supported. The problem rather seems to be in the caching of non-annotated classes: Since we're using the bean class as a key, proxies implementing the same interface will only be evaluated once - even if backed by different target classes. Could this be the case in your application? Some Processor implementations featuring an @Scheduled annotation while others don't?

Fixing this should be straightforward: Using the target class as a key in case of a proxy bean. I'm going to have a look at this for 4.1.5 (to be released next week) but I'm afraid a 3.2.x backport is still far out (since the 3.2.14 maintenance release isn't planned before mid 2015)...

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 12, 2015

Trent Summerfield commented

Thanks for looking at this. In the meantime we'll be hoisting the @Scheduled tasks up to a bean above the proxy to work around this. Once you have the fix committed on the 3.2.x branch let me know and I would be happy to build it from source and confirm it is fixed.

Trent

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 12, 2015

Trent Summerfield commented

Could this be the case in your application? Some Processor implementations featuring an @Scheduled annotation while others don't?

Yes this is exactly our situation. Processor and the aspect provide logging but not all processors are @Scheduled some are called through controllers or other means.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 12, 2015

Juergen Hoeller commented

Alright, chances are high that my local fix towards the target class as cache key will resolve your problem then. I'll backport it to the 3.2.x branch tomorrow; no need to build from source, since our CI build for the 3.2.x branch produces a snapshot for each push.

As for a potential 3.2.14 release date, we can advance it to mid May when 4.2 RC1 and 4.1.6 are scheduled to go live. For the time being, I can only offer a fix in 4.1.5 which is going out any day now. The main 'problem' is that we have hardly any other fixes which apply to 3.2.14 yet, and we're only going to release it with a few more issues resolved.

BTW, any specific reason why you're still on 3.2.x? Any showstoppers for an upgrade to 4.1.x? Just wondering, since we intend to let 3.2.x reach its end-of-life by the end of this year...

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 12, 2015

Trent Summerfield commented

No we have multiple applications on spring 4.1.x and are happy with it. But we also have the dusty old ball of mud application that is still on 3.2.x. If I know it is being end-of-life then I can push through the upgrade.

Trent

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 18, 2015

Juergen Hoeller commented

Trent, this is finally available in a 3.2.14 snapshot now:

<dependencies>
    <dependency>
        <groupId>org.springframework</groupId>
        <artifactId>spring-context</artifactId>
        <version>3.2.14.BUILD-SNAPSHOT</version>
    </dependency>
</dependencies><repositories>
    <repository>
        <id>spring-snapshots</id>
        <name>Spring Snapshots</name>
        <url>http://repo.spring.io/snapshot</url>
        <snapshots>
            <enabled>true</enabled>
        </snapshots>
    </repository>
</repositories>

Please give it a try if you have the chance, ideally before the 4.1.5 release (scheduled for Friday)!

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 18, 2015

Trent Summerfield commented

Just confirmed this does indeed fix the problem. Thanks for the quick response to this, I appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.