OF-3176: Offload blocking Netty handlers to dedicated executor

[guusdk]

Introduce a shared blocking handler executor for Netty pipelines to ensure that potentially blocking or long-running operations do not execute on EventLoop threads.

This change separates responsibilities between:

• acceptor (boss) EventLoopGroup
• non-blocking I/O worker EventLoopGroup
• dedicated executor for blocking pipeline handlers

The new executor is shared across all channels created by a NettyConnectionAcceptor and is used for handlers that may perform authentication, routing, persistence, or other blocking work.

This improves throughput and stability under load by preventing EventLoop starvation and aligns Openfire’s Netty usage with Netty best practices.

The connection configuration’s "max thread pool size" is now applied to the dedicated blocking handler executor. Netty EventLoopGroups (acceptor and I/O worker) now use Netty’s default thread sizing, as they are reserved exclusively for non-blocking socket I/O and protocol framing.

[Copilot]

Pull request overview

This PR introduces a dedicated EventExecutorGroup for potentially blocking Netty pipeline handlers, to keep Netty EventLoop threads focused on non-blocking I/O and reduce EventLoop starvation under load.

Changes:

• Add a shared blocking-handler executor to NettyConnectionAcceptor and run core “business logic” handlers on that executor.
• Extend plugin handler injection to receive an executor, enabling plugins to offload their handlers from EventLoop threads.
• Update outbound S2S session initialization to also offload its business logic handler to a dedicated executor, plus documentation updates describing the new threading model.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
xmppserver/src/main/java/org/jivesoftware/openfire/spi/NettyServerInitializer.java	Offloads the core business logic handler to a provided blocking executor; passes executor to plugin handler factories.
xmppserver/src/main/java/org/jivesoftware/openfire/spi/NettyConnectionAcceptor.java	Introduces and manages lifecycle of a shared blocking handler executor; updates bootstrap wiring and handler factory calls.
xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettySessionInitializer.java	Offloads outbound S2S business logic to a new executor; introduces per-instance executor and thread naming changes.
xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyChannelHandlerFactory.java	Changes plugin extension interface to accept an executor for handler execution.
documentation/internal-networking.html	Documents the new dedicated executor concept alongside boss/worker groups.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[guusdk]

Does this solution void the fixes for https://igniterealtime.atlassian.net/browse/OF-3180 in #3140 ?

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Direct TLS startup calls ch.attr(CONNECTION).get().startTLS(true, true) immediately after adding NettyOutboundConnectionHandler with blockingHandlerExecutor. Because handlerAdded for that handler is executed asynchronously on the executor group, the CONNECTION attribute may still be null here, which can crash outbound S2S connection establishment. Defer TLS initialization until the NettyConnection has been created (eg, after handlerAdded completes, preferably by scheduling pipeline mutation on the channel EventLoop).

[Copilot]

If createStanzaHandler(...) (or subsequent attr assignments) throws, stanzaHandlerFuture is never completed, which can cause callers waiting on getStanzaHandlerFuture() to hang indefinitely. Wrap stanza handler creation/assignment in try/catch and complete stanzaHandlerFuture exceptionally (and/or close the channel) on failure.

[Copilot]

NettySessionInitializer shared resources are started here unconditionally for every ConnectionListener. This should only run for the S2S listener (ConnectionType.SOCKET_S2S); otherwise starting any listener (eg C2S) unnecessarily starts outbound S2S resources and makes their lifecycle dependent on unrelated listeners.

[Copilot]

Direct TLS startup calls ch.attr(CONNECTION).get().startTLS(...) immediately after adding businessLogicHandler with blockingHandlerExecutor. Because handlerAdded for that handler is executed asynchronously on the executor group, the CONNECTION attribute may not be set yet, leading to a NullPointerException/race at startup. Defer startTLS until after handlerAdded has created and stored the NettyConnection (for example, trigger it from within handlerAdded or via a future that completes when CONNECTION is available, and run the TLS pipeline mutation on the channel EventLoop).

[Copilot]

Auto-read is re-enabled by scheduling a task on blockingHandlerExecutor during channelRegistered. Because businessLogicHandler was added with that same executor group, its handlerAdded (which initializes CONNECTION/HANDLER attributes) also runs asynchronously on the pool; with multiple threads there is no ordering guarantee, so reads can be enabled before initialization completes. Re-enable autoRead only after businessLogicHandler signals initialization completion (eg via a future) and perform setAutoRead(true) on the channel EventLoop.

[Copilot]

Auto-read is re-enabled by scheduling a task on blockingHandlerExecutor during channelRegistered. Since NettyOutboundConnectionHandler was added with that executor group, its handlerAdded (which sets CONNECTION/HANDLER) also runs asynchronously on the pool; there is no sequencing guarantee, so reads can be enabled before initialization completes. Consider enabling autoRead only after handler initialization completes (eg via getStanzaHandlerFuture()) and execute the config change on the channel EventLoop.

Suggested change

	// Re-enable autoRead after the channel is fully registered.
	ch.pipeline().addLast(new ChannelInboundHandlerAdapter() {
	@Override
	public void channelRegistered(ChannelHandlerContext ctx) {
	// Schedule enabling auto-read on the blocking executor to ensure pipeline is fully ready.
	blockingHandlerExecutor.execute(() -> ctx.channel().config().setAutoRead(true));
	ctx.fireChannelRegistered();
	// Re-enable autoRead only after the outbound handler has completed its initialization,
	// and perform the config change on the channel's EventLoop.
	ch.pipeline().addLast(new ChannelInboundHandlerAdapter() {
	@Override
	public void channelRegistered(ChannelHandlerContext ctx) {
	ctx.fireChannelRegistered();
	final Channel channel = ctx.channel();
	final CompletableFuture<?> initFuture = businessLogicHandler.getStanzaHandlerFuture();
	if (initFuture != null) {
	initFuture.whenComplete((result, error) -> {
	if (error == null && channel.isOpen()) {
	channel.eventLoop().execute(() -> {
	if (channel.isOpen()) {
	channel.config().setAutoRead(true);
	}
	});
	}
	});
	} else {
	// Fallback: if no init future is available, still enable auto-read on the EventLoop.
	channel.eventLoop().execute(() -> {
	if (channel.isOpen()) {
	channel.config().setAutoRead(true);
	}
	});
	}

[Copilot]

readyLatch.await() has no timeout. If the boss EventLoop cannot execute the barrier task for any reason (shutdown, deadlock, unexpected scheduling failure), start() can hang indefinitely. Consider using a bounded await with logging/failure handling (and restore the interrupt flag when interrupted) to avoid blocking startup forever.

[Copilot]

NettySessionInitializer shared resources are stopped here unconditionally. ConnectionListener is used for multiple connection types, so stopping/restarting a non-S2S listener would tear down shared outbound S2S Netty pools and disrupt outbound federation. Gate stopSharedResources() behind a check for ConnectionType.SOCKET_S2S (or move lifecycle management to the S2S subsystem).

[Copilot]

Same issue on shutdown: stop() always invokes NettySessionInitializer.stopSharedResources(), which will tear down outbound S2S resources even when stopping a non-S2S listener. This should be conditional on getType() == ConnectionType.SOCKET_S2S (or otherwise managed by the S2S subsystem) to avoid breaking outbound S2S during unrelated listener lifecycle events.

Suggested change

	connectionAcceptor.stop(); // Stop accepting inbound S2S connections.
	NettySessionInitializer.stopSharedResources(); // Stop the shared resources for outbound S2S connections.
	connectionAcceptor.stop(); // Stop accepting inbound connections for this listener.
	if ( getType() == ConnectionType.SOCKET_S2S )
	{
	NettySessionInitializer.stopSharedResources(); // Stop the shared resources for outbound S2S connections.
	}