[PR] Prevent RabbitMQ channel pool starvation, event bus listener leak, and webadmin resource leaks on cancellation and error [james-project]

["prosgarz35 (via GitHub)" <gi...@apache.org>]

prosgarz35 opened a new pull request, #3239:
URL: https://github.com/apache/james-project/pull/3239

   ## Overview
   
   This PR fixes multiple high-severity resource leak vulnerabilities across RabbitMQ connection pooling, health checks, and WebAdmin maintenance routes:
   
   1. **RabbitMQ Channel Pool Starvation (`ReactorRabbitMQChannelPool.java`)**:
      - `tryChannel()`, invoked by `RabbitMQHealthCheck` periodically, borrowed a channel from the reactive channel pool via `borrow()`.
      - In `Mono.usingWhen(borrow(), channel -> Mono.just(channel.isOpen()), ...)` only the success path returned the borrowed channel back to the pool (`pooledRef.release()`).
      - If downstream execution was cancelled (e.g. by a health check timeout) or encountered an error, the borrowed channel was neither released nor invalidated, leaking permanently into `refs`.
      - Under real workloads, this led to rapid channel pool exhaustion (`Timeout waiting for idle object`), completely blocking queue publishing and dequeuing.
      - **Fix**: Added explicit `onError` and `onCancel` callbacks to release or invalidate the channel.
   
   2. **EventBus Listener Memory Leak on HealthCheck Timeout (`MailReceptionCheck.java`)**:
      - In `MailReceptionCheck.check()`, a temporary `AwaitReceptionListener` was registered on the `eventBus`.
      - When mail reception timed out (`.timeout(...)`), the reactive stream was cancelled, but `Registration::unregister` was only hooked into `onComplete`.
      - This caused leaked listeners to indefinitely accumulate in memory on the `eventBus`, causing memory bloat and degrading event dispatch performance.
      - **Fix**: Added `(registration, error) -> registration.unregister()` and `Registration::unregister` for cancellation cleanup.
   
   3. **Temporary Blob Leak during Mailbox Export (`ExportService.java`)**:
      - In `ExportService.export()`, user mailboxes are zipped and temporarily staged in the BlobStore (`blobStore.save(...)`).
      - `deleteBlob` was only executed upon successful export completion. If the export failed or was cancelled via WebAdmin Tasks API, the temporary blob was never deleted, leaving orphan blobs in S3 / file / Cassandra storage.
      - **Fix**: Added error and cancellation handlers to ensure `deleteBlob` is always executed.
   
   4. **Stream & File Descriptor Leak during Message Vault Restore (`RestoreService.java`)**:
      - In `RestoreService.appendToMailbox()`, nested `Mono.usingWhen` calls handled the MIME `InputStream` and `ByteSourceContent`.
      - Neither of the two nested `usingWhen` calls closed resources on error or cancellation.
      - **Fix**: Added `onError` and `onCancel` close handlers to both `usingWhen` blocks.
   
   ---
   
   ## Changes by Module
   
   - **`backends-common/rabbitmq`**:
     - `org.apache.james.backends.rabbitmq.ReactorRabbitMQChannelPool`:
       - Extracted channel release logic into `releaseChannel(Channel)` that checks channel status and executes `pooledRef.release()` or `pooledRef.invalidate()`.
       - Wired `releaseChannel` to `onError` and `onCancel` in `tryChannel()`.
   
   - **`server/container/feature-checks`**:
     - `org.apache.james.healthcheck.MailReceptionCheck`:
       - Added error and cancellation handlers to unregister the `eventBus` listener.
   
   - **`server/protocols/webadmin/webadmin-mailbox`**:
     - `org.apache.james.webadmin.service.ExportService`:
       - Added error and cancellation handlers to delete staged blobs on failure or task abort.
   
   - **`server/protocols/webadmin/webadmin-mailbox-deleted-message-vault`**:
     - `org.apache.james.webadmin.vault.routes.RestoreService`:
       - Added error and cancellation handlers to close both the underlying message `InputStream` and the `ByteSourceContent` temporary wrapper.
   
   ---
   
   ## Verification
   
   The fixes were compiled and verified on JDK 21:
   - `backends-common/rabbitmq`: `mvn test-compile` - **SUCCESS**
   - `server/container/feature-checks`: `mvn test-compile` - **SUCCESS**
   - `server/protocols/webadmin/webadmin-mailbox`: `mvn test-compile` - **SUCCESS**
   - `server/protocols/webadmin/webadmin-mailbox-deleted-message-vault`: `mvn test-compile` - **SUCCESS**
   - `server/apps/distributed-app`: `mvn test-compile` - **SUCCESS**
   - `server/apps/postgres-app`: `mvn test-compile` - **SUCCESS**


-- 
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: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org

Re: [PR] Prevent RabbitMQ channel pool starvation, event bus listener leak, and webadmin resource leaks on cancellation and error [james-project]

["chibenwa (via GitHub)" <gi...@apache.org>]

chibenwa commented on PR #3239:
URL: https://github.com/apache/james-project/pull/3239#issuecomment-6128054343

   Hello @prosgarz35 
   
   > The only meaningful change is in ReactorRabbitMQChannelPool.tryChannel(): invalidating a closed channel instead of releasing it, plus the null guard.
   >
   > Could you reduce the PR to that, ideally sharing the logic with getChannelCloseHandler() and adding a test?
   
   Would you have time to rework this PR in order to address the only real issue mentionned here?
   
   Thanks,
   
   Best regards,
   
   Benoit


-- 
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: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org