OF-3179 & OF-3180: Safe, ordered, and async ConnectionCloseListener execution#3140
Merged
akrherz merged 4 commits intoigniterealtime:mainfrom Feb 16, 2026
Conversation
Expand the listener Javadoc to clearly document that a connection closure does not necessarily imply session closure. Add guidance warning implementers not to make this assumption and direct them to SessionEventListener for reliable session lifecycle events.
Member
|
@gemini-code-assist please review |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Openfire’s connection-close handling to execute ConnectionCloseListener callbacks deterministically (priority order) and off the Netty event loop, using a dedicated executor managed by ConnectionManagerImpl.
Changes:
- Introduces a dedicated “connection event”
ThreadPoolExecutorinConnectionManagerImpl, including configurable system properties and JMX exposure. - Updates close-listener invocation to run sequentially by priority and to be offloaded to the new executor.
- Adjusts built-in listeners to return already-completed futures and to declare built-in priority.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
xmppserver/src/main/java/org/jivesoftware/openfire/spi/ConnectionManagerImpl.java |
Adds a dedicated executor + async helper APIs, system properties, and JMX registration/unregistration. |
xmppserver/src/main/java/org/jivesoftware/openfire/net/AbstractConnection.java |
Changes close listener storage and implements priority-ordered, executor-offloaded, sequential listener execution. |
xmppserver/src/main/java/org/jivesoftware/openfire/SessionManager.java |
Updates built-in close listeners to run synchronously (on the offloaded executor) and assigns built-in priority. |
xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalComponentSession.java |
Updates component session close listeners to synchronous + built-in priority. |
xmppserver/src/main/java/org/jivesoftware/openfire/net/ComponentStanzaHandler.java |
Updates extra-domain component close listener to synchronous + built-in priority. |
xmppserver/src/main/java/org/jivesoftware/openfire/ConnectionCloseListener.java |
Adds priority concept (constants + default method) and clarifies semantics in Javadoc. |
i18n/src/main/resources/openfire_i18n.properties |
Adds i18n strings for the new executor system properties. |
i18n/src/main/resources/openfire_i18n_nl.properties |
Adds Dutch i18n strings for the new executor system properties. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
xmppserver/src/main/java/org/jivesoftware/openfire/spi/ConnectionManagerImpl.java
Outdated
Show resolved
Hide resolved
xmppserver/src/main/java/org/jivesoftware/openfire/net/AbstractConnection.java
Show resolved
Hide resolved
xmppserver/src/main/java/org/jivesoftware/openfire/spi/ConnectionManagerImpl.java
Outdated
Show resolved
Hide resolved
xmppserver/src/main/java/org/jivesoftware/openfire/spi/ConnectionManagerImpl.java
Show resolved
Hide resolved
guusdk
force-pushed
the
OF-3180_OF-3179_Connection-closure-improvements
branch
from
41303b1 to
8a0b927
Compare
Member
Author
|
@copilot retest |
Member
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
xmppserver/src/main/java/org/jivesoftware/openfire/spi/ConnectionManagerImpl.java
Outdated
Show resolved
Hide resolved
xmppserver/src/main/java/org/jivesoftware/openfire/spi/ConnectionManagerImpl.java
Outdated
Show resolved
Hide resolved
xmppserver/src/main/java/org/jivesoftware/openfire/spi/ConnectionManagerImpl.java
Outdated
Show resolved
Hide resolved
xmppserver/src/main/java/org/jivesoftware/openfire/net/AbstractConnection.java
Show resolved
Hide resolved
xmppserver/src/main/java/org/jivesoftware/openfire/net/AbstractConnection.java
Show resolved
Hide resolved
guusdk
force-pushed
the
OF-3180_OF-3179_Connection-closure-improvements
branch
from
8a0b927 to
84fd434
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…read pool Add a dedicated thread pool executor to ConnectionManagerImpl for processing connection events asynchronously. The notifyCloseListeners() method in AbstractConnection now dispatches listener invocations on this executor rather than invoking them on the calling thread. This prevents blocking or long-running listener implementations (e.g., those performing database or network I/O) from interfering with connection shutdown or I/O processing. While all connection types benefit, this is particularly important for Netty-based connections where blocking the event loop thread can cause widespread connection stalls. The new thread pool is configurable via system properties: - `xmpp.connectionmanager.eventexecutor.threads` (default: 16) - `xmpp.connectionmanager.eventexecutor.timeout` (default: 60s)
…ispatcher The dispatcher for ConnectionCloseListener events was updated in a previous commit (for OF-3180) to ensure all listener invocations are executed asynchronously and offloaded from the calling thread. This change allows for significant simplification of the listener implementations, as they no longer need to manually wrap their logic in CompletableFuture.runAsync or thenRunAsync. This commit removes redundant async wrapping in all ConnectionCloseListener implementations and uses the same executor for all async work.
Ensures consistency in listener execution. Critical cleanup runs before third-party listeners to prevent stale session state. - Added `getPriority()` to `ConnectionCloseListener` (default: 0, built-in: 100) - Updated `notifyCloseListeners()` to sort and execute listeners sequentially by priority - Preserves insertion order for same-priority listeners
guusdk
force-pushed
the
OF-3180_OF-3179_Connection-closure-improvements
branch
from
84fd434 to
f37cd69
Compare
Fishbowler
approved these changes
Feb 16, 2026
Member
Fishbowler
left a comment
Fishbowler
left a comment
There was a problem hiding this comment.
This all seems logical. I love the priority impl, and love the clean-up of nested Futures.
The tests running gives some confidence that it's not utterly broken. But I think dogfooding makes sense to really prove it.
akrherz
approved these changes
Feb 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR ensures ConnectionCloseListeners run in a deterministic priority order (fixing OF-3179) and offloads all listener execution from Netty event loops (fixing OF-3180), preventing stale session state and thread starvation.
Key Changes:
Please see individual commit messages for additional details.