Skip to content

Adjust ThreadPoolTaskScheduler bean registration to use Integer.MAX_VALUE / 2 phase #8856

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

Closed
artembilan opened this issue Jan 12, 2024 · 0 comments

Comments

@artembilan
Copy link
Member

Starting with Spring Framework 6.1 the ExecutorConfigurationSupport now supports a lifecycle management.
It comes with a private int phase = DEFAULT_PHASE;.
It has this logic:

	/**
	 * Pause this executor, triggering the given callback
	 * once all currently executing tasks have completed.
	 * @since 6.1
	 */
	@Override
	public void stop(Runnable callback) {
		if (this.lifecycleDelegate != null && !this.lateShutdown) {
			this.lifecycleDelegate.stop(callback);
		}
		else {
			callback.run();
		}
	}

So, it waits for scheduled tasks to be finished.
The AbstractPollingEndpoint uses Integer.MAX_VALUE / 2 and calls this.runningTask.cancel(true); in its stop(). So, no new messages are emitted to the application.
However this combination leads to dead-lock and unexpected behavior.
According to the phase logic

	 * The default phase for {@code SmartLifecycle}: {@code Integer.MAX_VALUE}.
	 * <p>This is different from the common phase {@code 0} associated with regular
	 * {@link Lifecycle} implementations, putting the typically auto-started
	 * {@code SmartLifecycle} beans into a later startup phase and an earlier
	 * shutdown phase.

The ThreadPoolTaskScheduler is stopped earlier, then PollingConsumer.
And the former waits for its tasks to be finished, not giving the later a chance to cancel its task.

Therefore the ThreadPoolTaskScheduler configuration in the DefaultConfiguringBeanFactoryPostProcessor.registerTaskScheduler() should be adjusted to use the same Integer.MAX_VALUE / 2 phase.

@artembilan artembilan added this to the 6.3.0-M1 milestone Jan 12, 2024
artembilan added a commit that referenced this issue Jan 12, 2024
Fixes: #8856

The `ThreadPoolTaskScheduler` in Spring Framework manages now a lifecycle
with a `Integer.MAX_VALUE` phase by default.
This causes a wait block for scheduled tasks in the `AbstractPollingEndpoint` instances.
These tasks are cancelled by in the later `Integer.MAX_VALUE / 2` phase.
So, we have a lifecycle deadlock on context close

* Fix `DefaultConfiguringBeanFactoryPostProcessor.registerTaskScheduler()`
with a `.addPropertyValue("phase", SmartLifecycle.DEFAULT_PHASE / 2)`
to let the `AbstractPollingEndpoint` to cancel its currently scheduled task
to avoid possible pollution with unexpected (and lost) messages

**Cherry-pick to `6.2.x`**

(cherry picked from commit 11e0ea1)
artembilan added a commit that referenced this issue Jan 16, 2024
Related to: #8856

Many tests create their own `ThreadPoolTaskScheduler` beans.
Therefore, its default phase might affect the memory and performance.

* Use `phase = SmartLifecycle.DEFAULT_PHASE / 2` for manual
 `ThreadPoolTaskScheduler` beans
* Migrate affected tests classes to JUnit 5
* Make some other configuration adjustments for better performance

**Cherry-pick to `6.2.x`**
artembilan added a commit that referenced this issue Jan 16, 2024
Related to: #8856

Many tests create their own `ThreadPoolTaskScheduler` beans.
Therefore, its default phase might affect the memory and performance.

* Use `phase = SmartLifecycle.DEFAULT_PHASE / 2` for manual
 `ThreadPoolTaskScheduler` beans
* Migrate affected tests classes to JUnit 5
* Make some other configuration adjustments for better performance

**Cherry-pick to `6.2.x`**

(cherry picked from commit 39c99c0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants