prosgarz35 opened a new pull request, #3198:
URL: https://github.com/apache/james-project/pull/3198
# IMAP IDLE Improvements: Non-blocking Event Synchronization & Heartbeat
Lifecycle Safety
## 1. Context & Motivation
Apache James has been progressively migrating its protocol processing stack
to a non-blocking reactive model using **Project Reactor**.
However, the IMAP `IDLE` implementation in `IdleProcessor` still contained
legacy blocking synchronization primitives and unhandled lifecycle edge cases.
Under production workloads with concurrent mail delivery and unstable client
network connectivity (especially mobile clients like iOS Mail, Android K-9,
etc.), these issues caused:
1. **Thread Starvation**: Exhaustion of Reactor/Netty worker threads due to
blocking calls inside reactive event callbacks.
2. **Resource & Timer Leaks**: Zombie heartbeat tasks remaining scheduled
and retaining session references when sockets drop unexpectedly.
This document describes the problems identified and the rationale behind the
targeted, low-risk fixes.
---
## 2. Issues Identified
### Issue 1: Blocking `CountDownLatch::await` Inside Reactive Event Pipeline
- **Location**:
`org.apache.james.imap.processor.IdleProcessor.IdleMailboxListener#reactiveEvent`
- **Symptom**:
```java
@Override
public Publisher<Void> reactiveEvent(Event event) {
return Mono.fromRunnable(Throwing.runnable(countDownLatch::await))
.then(Mono.defer(() -> unsolicitedResponses(session, responder,
false)))
.then(Mono.fromRunnable(responder::flush));
}
```
- **Root Cause**:
`CountDownLatch` was used to ensure that mailbox change events (such as
`Added`, `Expunged`, `FlagsUpdated`) would not trigger `unsolicitedResponses`
until the initial `IDLE` handshake (`+ idling` response and initial untagged
updates) completed.
However, `reactiveEvent` runs on reactive scheduler threads (such as
`reactor-boundedElastic-*` or Netty worker event loops). Invoking
`countDownLatch::await` blocks the OS thread.
- **Impact**:
During bursts of incoming emails across multiple IDLE sessions, threads in
the pool are held up waiting on the latch. This leads to thread pool
starvation, causing latency spikes and connection timeouts across unrelated
IMAP commands (e.g. `LOGIN`, `FETCH`).
---
### Issue 2: Unchecked Heartbeat Rescheduling on Broken/Half-Closed
Connections
- **Location**: `org.apache.james.imap.processor.IdleProcessor#idle`
- **Symptom**:
The heartbeat task checked only `session.getState() !=
ImapSessionState.LOGOUT && idleActive.get()`.
When a remote client disconnects abnormally (e.g. cellular network drop,
NAT timeout without a clean TCP FIN/RST):
- The session state may not transition to `LOGOUT` immediately.
- Writing the untagged `* OK Still here` heartbeat can fail or throw an
exception.
- If unhandled or uncaught, either the exception halts the handler without
proper cleanup, or recursive rescheduling continues to retain references to
`ImapSession`, `Responder`, and associated channel buffers.
- **Impact**:
Gradual heap memory retention and unnecessary scheduling overhead for dead
client connections.
---
## 3. Proposed & Applied Solutions
Both changes are strictly localized to `IdleProcessor.java`, preserve 100%
backward compatibility, and avoid touching public APIs.
### Fix 1: Replace `CountDownLatch` with Reactive `Sinks.One<Void>`
- Instead of a blocking synchronization primitive, use `Sinks.One<Void>
idleReadySink = Sinks.one()`.
- Trigger completion when initialization finishes:
```java
return Mono.fromRunnable(() -> idle(request, session, responder,
idleReadySink))
.then(unsolicitedResponses(session, responder, false))
.onErrorResume(...)
.doFinally(signalType -> idleReadySink.tryEmitEmpty());
```
- In `IdleMailboxListener#reactiveEvent`, replace the blocking call with
non-blocking reactive subscription:
```java
@Override
public Publisher<Void> reactiveEvent(Event event) {
return idleReadySink.asMono()
.then(Mono.defer(() -> unsolicitedResponses(session, responder,
false)))
.then(Mono.fromRunnable(responder::flush));
}
```
- **Benefit**: Retains identical event sequencing guarantees, but **never
blocks the underlying execution thread**.
### Fix 2: Safeguard Heartbeat Execution and Rescheduling
- Wrap the heartbeat send operation in a defensive `try-catch` block.
- If writing to the responder fails due to a disconnected/broken pipe,
immediately mark `idleActive.set(false)` and abort further scheduling.
- Re-verify `idleActive.get() && session.getState() !=
ImapSessionState.LOGOUT` right before invoking `session.schedule(this,
heartbeatInterval)`.
- **Benefit**: Ensures self-healing termination of heartbeat tasks upon
connection failure, preventing memory and timer leaks.
---
## 4. Verification & Testing
- Verified with Maven 3.9+ and OpenJDK:
- Clean compilation of module `protocols-imap`.
- Zero Checkstyle violations (`checkstyle:check passed`).
- No changes to public interfaces or behavioral contract with IMAP clients.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
