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

@JmsListener with autoStartup=false will never start [SPR-14105] #18677

Closed
spring-issuemaster opened this issue Apr 1, 2016 · 4 comments
Closed
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Apr 1, 2016

Gary Russell opened SPR-14105 and commented

JmsListenerEndpointRegistry.start() delegates to startIfNecessary() which does not start the container if autoStartup is false.

Hence, initializing with auto startup false, means the containers can never start.

Work around is to loop over the containers, and change the autoStartup before calling start().

for (MessageListenerContainer container : registry.getListenerContainers()) {
	((AbstractJmsListeningContainer) container).setAutoStartup(true);
}

Or you can iterate over the containers to start instead of invoking registry.start().


Affects: 4.2.5

Reference URL: http://stackoverflow.com/questions/36332914/start-and-stop-jmslistener/36335015#comment60311685_36335015

Issue Links:

  • AMQP-595 @RabbitListener with autoStartup=false will never start
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 4, 2016

Stéphane Nicoll commented

I am still confused by that bug report to be honest.

Containers are created by the registry. Since containers are no Spring Beans (see the class Javadoc), the registry makes sure to honour the container lifecycle callback. I had a look to the related stackoverflow thread and what you suggested as a workaround is exactly what the user should be doing IMO.

If you configure autoStartup to false, it means you want to control when the containers start. The way you would do it previously would be to inject the container in some management bean and call start on it. What is the reason to do things otherwise? I had a look to the change in Rabbit and I don't like it, start has two different behaviours now.

What am I missing Gary Russell?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 4, 2016

Gary Russell commented

Stéphane Nicoll The real issue is inconsistency with when the initial value is true and the fact that the SmartLifecycle contract is not really honored by the registry.

The fact that start() delegates to the containers it holds is not really important; it works differently when the containers' autoStartup are true. In that case, start and stop on the registry works fine; if it's false, stop() on the registry works but start() does not. IMHO, start() should always work as expected (with the same semantics as autoStartup) when called explicitly.

I think the real flaw is that the registry itself always returns true for auto-startup; that is what should control whether the containers are started when the context is refreshed. i.e. the lifecycle of the registered containers should be controlled by the lifecycle of the registry, the registry should not care about the delegate states - as you said, they are not beans.

This is how I fixed it for @RabbitListener and @KafkaListener, but I think the correct fix is to make the registry properly comply with the SmartLifecycle contract.

One more point - I think users would have a reasonable expectation that context.start() will start everything, including anything that has autoStartup=false - which is how it works for everything else.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 4, 2016

Stéphane Nicoll commented

I think users would have a reasonable expectation that context.start() will start everything, including anything that has autoStartup=false - which is how it works for everything else.

Good point, thanks!

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 4, 2016

Stéphane Nicoll commented

This is fixed in master and will be available in the next snapshot

Juergen Hoeller could you please review and backport to 4.2.6 accordingly? Thanks!

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