DefaultMessageListenerContainer doesn't shutdown gracefully if long recovery interval is set [SPR-14200]

[spring-projects-issues]

Oleg Ivanov opened SPR-14200 and commented

Currently in DefaultMessageListenerContainer.applyBackOffTime(BackOffExecution execution) function the current thread is sleeping during the back off interval. This prevents container to shutdown correctly (as it waits for this thread to wake up).

I suggest to rewrite this function using .wait() calls:

        final long interval = execution.nextBackOff();
        if (interval == BackOffExecution.STOP)
            return false;

        if (isRecovering() && interrupted)
            // Interrupted right before and still failing... give up.
            return false;

        try {
            synchronized (this.lifecycleMonitor) {
                final long start = System.currentTimeMillis();
                while (isActive() && System.currentTimeMillis() - start < interval) {
                    final long timeout = getReceiveTimeout();
                    this.lifecycleMonitor.wait(timeout > 0 ? timeout : DEFAULT_RECEIVE_TIMEOUT);
                }
            }
        } catch (InterruptedException interEx) {
            // Re-interrupt current thread, to allow other threads to react.
            Thread.currentThread().interrupt();
            if (isRecovering())
                this.interrupted = true;
        }

        return true;

---

Affects: 4.2.5

Issue Links:

• DefaultMessageListenerContainer hangs on shutdown [SPR-11787] #16409 DefaultMessageListenerContainer hangs on shutdown
• DefaultMessageListenerContainer#doShutdown hangs [SPR-11841] #16460 DefaultMessageListenerContainer#doShutdown hangs
• Client-side polling delay as alternative for receive timeout for JMS [SPR-14225] #18799 Client-side polling delay as alternative for receive timeout for JMS

Referenced from: commits 6ab8d36

[spring-projects-issues]

Juergen Hoeller commented

To the best of my understanding, a simple switch to lifecycleMonitor.wait(recoveryInterval) should do the job, allowing for notifyAll to reach it. During a recovery phase, our invoker infrastructure just triggers such a notification in case of a stop call or a full shutdown, both of which turn isRunning to false... so if applyBackOffTime exits normally, the surrounding loop will automatically check isRunning and end the refresh attempt.

I've implemented this for 4.3 RC2. Feel free to give the upcoming 4.3.0.BUILD-SNAPSHOT an early try...

[spring-projects-issues]

Oleg Ivanov commented

Hi, Juergen, you are right - lifecycleMonitor.wait(interval) is enough. I didn't realize this monitor can be used. Thanks.